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

Added commit mode #139

Merged
merged 3 commits into from Dec 13, 2018
Merged

Added commit mode #139

merged 3 commits into from Dec 13, 2018

Conversation

sushilicious
Copy link

I found that when Instaloader is interrupted (ctr-c, computer crash, etc) in the middle of a run, it may not correctly save the last picture. This is a commit mode that I scrapped up to make sure Instaloader will redownload the corrupted picture in its next run.

The commit mode ensures pictures are not corrupted when Instaloader is
unexpectedly interrupted. In the case that the last picture is corrupted
because of an interruption, Instaloader will redownload the picture
in the next run. Since the metadata is the last object saved to disk,
we can consider a post as "committed" if its json metadata file exists and
is not malformed. Instaloader should download any posts which are not
committed. Downside is commit mode requires metadata to be saved.

@aandergr aandergr requested a review from Thammus July 8, 2018 07:26
@aandergr aandergr added this to the Version 4.1 milestone Jul 8, 2018
@aandergr aandergr added the feature suggestion Feature suggestion label Jul 30, 2018
@aandergr aandergr modified the milestones: Version 4.1, Version 4.2 Aug 31, 2018
Copy link
Member

@aandergr aandergr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :) I don't see an urgent need for this, since I never had the problem it solves, but I think there is no reason not to add it to our next version. I've added a few inline comments.

@@ -276,6 +276,10 @@ def main():
help='Maximum number of connection attempts until a request is aborted. Defaults to 3. If a '
'connection fails, it can be manually skipped by hitting CTRL+C. Set this to 0 to retry '
'infinitely.')
g_how.add_argument('--commit-mode', action='store_true',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add the flag to docs/cli-option.rst

@@ -309,6 +313,9 @@ def main():
raise SystemExit("--no-captions and --post-metadata-txt or --storyitem-metadata-txt given; "
"That contradicts.")

if args.commit_mode and args.no_metadata_json:
raise SystemExit('Commit mode requires JSON metadata to be saved.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise SystemExit('Commit mode requires JSON metadata to be saved.')
raise SystemExit('--commit-mode cannot be used together with --no-metadata-json.')

Naming the exact flags might it make easier for the user to understand and solve the problem (however, this is just a suggestion).

@@ -51,3 +51,7 @@ class ConnectionException(InstaloaderException):

class TooManyRequestsException(ConnectionException):
pass


class NeedMetadataException(InstaloaderException):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is no need to introduce a new exception. Normally we would just use InvalidArgumentException.

@@ -128,6 +130,12 @@ def __init__(self,
else post_metadata_txt_pattern
self.storyitem_metadata_txt_pattern = '' if storyitem_metadata_txt_pattern is None \
else storyitem_metadata_txt_pattern
self.commit_mode = commit_mode
if self.commit_mode and not self.save_metadata:
raise NeedMetadataException("Commit mode requires JSON metadata to be saved.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise NeedMetadataException("Commit mode requires JSON metadata to be saved.")
raise InvalidArgumentException("Commit mode requires JSON metadata to be saved.")

I think there is no need to introduce a new exception for this

@aandergr aandergr assigned aandergr and unassigned Thammus Nov 6, 2018
sushilicious added 3 commits December 9, 2018 02:12
The commit mode ensures pictures are not corrupted when Instaloader is
unexpectedly interrupted. In the case that the last picture is corrupted
because of an interruption, Instaloader will redownload the picture.
Since the metadata is the last object saved to disk, we can consider a
post as "committed" if its json metadata file exists and is not
malformed. Instaloader should download any posts which are not
committed. Downside is commit mode requires metadata to be saved.
This is in addition to the other commit logic.
@sushilicious
Copy link
Author

I have rebased to 4.1.1 and incorporated the changes you suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature suggestion Feature suggestion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants