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

Resolve #7889 avoid 3 fetch calls to load a notebook #7895

Closed
wants to merge 3 commits into from

Conversation

ikiw
Copy link
Contributor

@ikiw ikiw commented Feb 18, 2020

  1. Resolve 3 checkpoint get requests invoked to load a single notebook #7889 to avoid 3 fetch calls to load a notebook.
  2. Refactored my old PR Resolve #7874 - stop too many fetch calls to docmanager-extension on … #7879 to use debouncer instead of boolean flags

References

#7889

Code changes

  1. listCheckpoints() gets invoked thrice when a notebook is loaded resulting in 3 checkpoint calls. The code change is to have an object- checkpointPromiseQueue that holds the promises with locaPath as key. If there is already a promise to be resolved, the pending promise object is returned back instead of creating one. And the promise reference to be cleared off from the checkpointPromiseQueue, when the promise if resolved/rejected via finally block. And as a result only 1 fetch call to checkpoint is triggered.

  2. Refactor of older PR Resolve #7874 - stop too many fetch calls to docmanager-extension on … #7879, to make use of Debouncer in this use case as it solves the purpose of executing the callback only once even though invoked multiple times and making the code more cleaner.

User-facing changes

Backwards-incompatible changes

No

@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@afshin
Copy link
Member

afshin commented Feb 19, 2020

Hello @ikiw! Thank you for working on this issue! I've been looking at it and thinking about some Poll conversations that came up in the last two weekly calls.

Our weekly calls are schedule at a difficult time for your time zone (Wednesdays at 17:00 UTC at https://calpoly.zoom.us/my/jupyter), so I know you may not be able to come to the call. But your issue ties in with that conversation, so I would like to discuss it on the call today. If you can make it, that would be wonderful. If not, I will post the outcome of our conversation in this issue as well.

cc: @blink1073 @jasongrout @saulshanabrook @wolfv

@ikiw
Copy link
Contributor Author

ikiw commented Feb 25, 2020

Will be creating a new PR for the same from a feature branch. As my master branch is messed up with merge issues.

@ikiw ikiw closed this Feb 25, 2020
@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Mar 27, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg:docmanager pkg:services status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 checkpoint get requests invoked to load a single notebook
2 participants