Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hard to tell what the problem is when there's errors importing files #460

Open
Cedric-Boucher opened this issue Oct 13, 2023 · 7 comments · May be fixed by #461
Open

Hard to tell what the problem is when there's errors importing files #460

Cedric-Boucher opened this issue Oct 13, 2023 · 7 comments · May be fixed by #461

Comments

@Cedric-Boucher
Copy link

For example, I imported a few videos from my phone, one of them said "error" when importing. I tried importing the same videos again, since they were all still in the folder of course, and this time all of them had "error" when importing. I have no idea if that one file actually imported, or why the other ones have an "error".

Surely we can make it more clear what is wrong, maybe categorizing the errors when printing them out, or at least putting more details in some log file and pointing to that file in the print to the command prompt?

@Cedric-Boucher
Copy link
Author

Cedric-Boucher commented Oct 13, 2023

so, in filesystem.py, there's a method process_checksum(). This method is supposed to return the checksum of the file to import, but can return None as well. If the file being imported already exists in the destination folder and has the same checksum, this method returns None.

The caller of this method is another method, process_file(), as pasted below

        checksum = self.process_checksum(_file, allow_duplicate)
        if(checksum is None):
            log.info('Original checksum returned None for %s. Skipping...' %
                     _file)
            return

This is all fine, we should skip the file if it has already been imported. However, it ends up reporting that file as having encountered an error on import, when this is not actually the case. It should either report that it was successfully imported, or that it was not imported because it already existed, not "error".

@Cedric-Boucher
Copy link
Author

Cedric-Boucher commented Oct 13, 2023

to continue the comment above,
The import() method in elodie.py calls process_file(), and this is supposed to return the destination path as well, but will return None if process_file() returned None, which is the case when the file to be imported has already been imported.
The import_file() method is called in the _import() method. When the destination path is None, _import() will make has_errors equal to True:

    for current_file in files:
        dest_path = import_file(current_file, destination, album_from_folder,
                    trash, allow_duplicates)
        result.append((current_file, dest_path))
        has_errors = has_errors is True or not dest_path

Which on the following lines causes the program to exit:

    if has_errors:
        sys.exit(1)

@Cedric-Boucher
Copy link
Author

ah, here it is. Just before the sys.exit(1) call, a Result object is used to write the error output to the command line. When a result with destination None is appended to the Result object, it is saved as being an "error". Then when the results are printed to the command line, it reports these files that have previously been imported, as being errors.

@Cedric-Boucher
Copy link
Author

A potential solution to this issue is to add a "class" of results in the Result object. Instead of only having "success" and "error", we could also have a "duplicate" or "already_existed" variable to keep track of files that weren't imported due to already existing in the destination folder. This way it would not be reported as an error, but rather a duplicate file that has not been imported again.

@Cedric-Boucher
Copy link
Author

Cedric-Boucher commented Oct 13, 2023

I am working on this issue in my fork of Elodie.

I have encountered something that I don't understand. There are some log.info() calls that print nothing when the debug option is enabled. Expected behavior is that it prints the info message, as other log.info() calls do.

Example in filesystem.py:

                print("TEST!!!")
                log.info('%s already at %s.' % (
                    _file,
                    checksum_file
                ))

my print works, but log.info prints nothing with debug enabled, which is not what is expected

@Cedric-Boucher
Copy link
Author

nevermind on that log.info thing. I'm just blind apparently

@Cedric-Boucher
Copy link
Author

I believe I have fixed this issue to my satisfaction. See: this commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant