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

fast_update not work with highlights/stories when in commit-mode #257

Closed
pjw91 opened this issue Feb 17, 2019 · 6 comments
Closed

fast_update not work with highlights/stories when in commit-mode #257

pjw91 opened this issue Feb 17, 2019 · 6 comments
Labels
bug Bug

Comments

@pjw91
Copy link

pjw91 commented Feb 17, 2019

Describe the bug
When downloading highlights/stories with the commit-mode and fast-update are on,
the fast-update has no effect.

To Reproduce
instaloader -F --highlights SOME_PROFILE --commit-mode --no-posts

Expected behavior
Skip those downloaded, as if commit-mode off.

Instaloader version
4.2.3

Additional context
The reason is that self._committed is never set in download_highlights and download_stories, and never reset to None after a post is processed (either downloaded or skipped).

That means:

  1. _committed is always None, which effectively disables commit-mode
  2. After processing a post, the flag is set to True/False, no matter what actually status of the next highlights/stories is.

After a quick review, it seems that add a line of code self._committed = self.check_if_committed(filename) at the top of download_pic fix this issue.
However, I think it should be add to download_stories/highlights or somewhere else.

@aandergr aandergr added the bug Bug label Feb 19, 2019
@aandergr
Copy link
Member

Thanks for reporting this and sharing your insights!

After a quick review, it seems that add a line of code self._committed = self.check_if_commited(filename) at the top of download_pic fix this issue.

That would check_if_committed for each item in a sidecar, though one check would be sufficient.

However, I think it should be add to download_stories/highlights or somewhere else.

Or download_storyitem, which is used both for downloading stories and highlights.

Also, I think passing this _committed state through a class variable is unclean, error-prone and blocking thread-parallelization - we may have merged the PR (#139) too early and probably should reimplement the commit mode. @sushilicious any ideas/thoughts about this?

@davispuh
Copy link
Contributor

This looks fixed/resolved to me, no ?

@aandergr
Copy link
Member

This looks fixed/resolved to me, no ?

No, there hasn't been any work on this.

@sushilicious
Copy link

Yeah sorry I've never used the stories or highlights feature, so I didn't think to implement the commit mode for that. As is mentioned in the first post, adding the check for committed in download_stories/highlights is probably the right thing to do.

@aandergr, sounds totally reasonable to change the _committed state to something else. The patch was mostly just a quick and dirty thing that I wrote up since the lack of a commit state was bothering me. If someone else plans to properly implement this feature, I also recommend using something other than the json metadata file as the flag for "already committed". That file can get quite large, so a different indicator might make more sense. For example, saving currently downloading pictures/videos to a temp file and renaming it after it is done, so uncommitted pics/videos do not enter your download folder.

@github-actions
Copy link

There has been no activity on this issue for an extended period of time. This issue will be closed after further 14 days of inactivity.

@github-actions github-actions bot added the stale Issue is inactive for a long time label Oct 23, 2019
@github-actions github-actions bot closed this as completed Nov 7, 2019
@aandergr aandergr added leave open Not closed automatically due to inactivity and removed stale Issue is inactive for a long time labels Jan 19, 2020
@aandergr aandergr reopened this Jan 19, 2020
aandergr added a commit that referenced this issue Jun 3, 2020
--commit-mode has many bugs, especially #257, #483.  For now we discourage using
it until it is reimplemented.
This was referenced Jun 3, 2020
aandergr pushed a commit that referenced this issue Jun 20, 2020
Rather than checking the json file to make sure posts have been
successfully downloaded, data is stored in a temporary file which
is renamed when downloading has finished, as suggested in #257.
@aandergr aandergr removed the leave open Not closed automatically due to inactivity label Jun 21, 2020
@aandergr
Copy link
Member

This has been fixed by #697.

Rather than checking the json file to make sure posts have been successfully downloaded, data is stored in a temporary file which is renamed when downloading has finished.

It is merged into upcoming/v4.5, which can be tried out with:

pip3 install --upgrade git+https://github.com/instaloader/instaloader@upcoming/v4.5

fireattack pushed a commit to fireattack/instaloader that referenced this issue Aug 15, 2020
Rather than checking the json file to make sure posts have been
successfully downloaded, data is stored in a temporary file which
is renamed when downloading has finished, as suggested in instaloader#257.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug
Projects
None yet
Development

No branches or pull requests

4 participants