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

Comment out the tedious resource synchronization code. #223

Merged
merged 4 commits into from
May 7, 2024

Conversation

boo-yee
Copy link

@boo-yee boo-yee commented May 1, 2024

Hi, Robert. The rate limit excpetion was often talked about earlier. Maybe we can have an improvement now.

I found that Nixnote sychronizes the resources twice, when syncing the notes, the resources are synced together in fact, but it seems that Nixnote syncs the resources one by one later, and I think this is the reason that the exception often appears. So I commented out the standalone resources sychronization code. I commented out instead of deleting them, because I am afraid that there are some side effects. I have tested with an account which has only a few notes and resources, and the notes can be synchronized normally, but I am not totally sure if the exception occurs when there are a large amount of notes.

It cannot walk around the limit totally, but it should possibly low down its occurrence. My trouble now is that I find it takes too much time to upload my faked large sized note content, so I hope someone who has a large amount of notes can help on it, if there is any. So please use the travis CI to build a binary file if possible for the convenience of testing. Thank you first.

For solving this issue, I referred to the code of evernote-back(an open source project too, for backuping up notes, rate limit excepton lessly happens on it), and found it syncs the note along with the resources, no standalone synchronization for the resources. That is to say, if there are 1k notes, each note contains two images, then Nixnote previously needed to make (a few more than)1k+2k api callings. If we sync the notes together with the resources, it will be only (a few more than)1k. Based on this estimation, I think we can reduce the rate limit exceptons.

I keep the changelog unchanged for now. I'll update it as soon as the testing is finished.

@boo-yee boo-yee marked this pull request as draft May 1, 2024 14:36
@boo-yee boo-yee marked this pull request as ready for review May 1, 2024 14:37
@boo-yee boo-yee marked this pull request as draft May 1, 2024 14:38
@boo-yee boo-yee marked this pull request as ready for review May 1, 2024 14:39
@boo-yee boo-yee marked this pull request as draft May 2, 2024 00:45
@robert7
Copy link
Owner

robert7 commented May 2, 2024

Hi @boo-yee,
thanks, I'll check then changes.

@boo-yee
Copy link
Author

boo-yee commented May 2, 2024

Sorry but I have not completed yet, maybe have to work for more time. :(

@boo-yee boo-yee marked this pull request as ready for review May 2, 2024 13:45
@boo-yee
Copy link
Author

boo-yee commented May 2, 2024

I updated the post, adding some details, and I think this pr is ready for reviewing now.

@boo-yee boo-yee changed the title Low down the occurences of the rate limit exceptions. Comment out the duplicated resource synchronization code. May 3, 2024
@boo-yee boo-yee changed the title Comment out the duplicated resource synchronization code. Comment out the tedious resource synchronization code. May 5, 2024
@robert7 robert7 merged commit 4e62724 into robert7:develop May 7, 2024
@robert7
Copy link
Owner

robert7 commented May 7, 2024

Hi,
I can't test the changes myself now, but it looks good, and I merged your changes to develop.

Unfortunately Travis build doesn't work now, as my free account expired, but I already wrote to Travis, hopefully they resume my account.
Robert

@boo-yee
Copy link
Author

boo-yee commented May 7, 2024

That's all right. We can still do it by ourselves by hand. Thanks for your work.

@boo-yee
Copy link
Author

boo-yee commented May 7, 2024

There is a parameter of getNote function called withResource. If it is set to true, the api caller will download the reource along with the note. that is why I think the commented out code is tedious.

@boo-yee
Copy link
Author

boo-yee commented May 7, 2024

It seems that the Travis CI works now and the latest AppImage has been built. Thanks again.

@robert7
Copy link
Owner

robert7 commented May 7, 2024

Travis account is renewed and should work until end of 2024 (then it would need another renew)

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

Successfully merging this pull request may close these issues.

2 participants