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

Reduce polling #812

Merged
merged 7 commits into from
Nov 7, 2020
Merged

Reduce polling #812

merged 7 commits into from
Nov 7, 2020

Conversation

fcollonval
Copy link
Member

This reduce refresh polling.

Refresh is blocked if:

  • Not in a git repository
  • The Git tab is hidden (this can be turned off with the setting refreshIfHidden)

@ianhi
Copy link
Collaborator

ianhi commented Oct 31, 2020

Is it possible to immediately start a refresh when the git tab is brought back into view? Otherwise it may end up being that you switch and then need to wait three seconds for a refresh

@ianhi
Copy link
Collaborator

ianhi commented Oct 31, 2020

This could also be used to implement the ability to ignore repositories (#641) There could a setting that contains a list of filepaths to ignore.

@fcollonval
Copy link
Member Author

Is it possible to immediately start a refresh when the git tab is brought back into view? Otherwise it may end up being that you switch and then need to wait three seconds for a refresh

It is a good suggestion. I'll try to find a way.

This could also be used to implement the ability to ignore repositories (#641) There could a setting that contains a list of filepaths to ignore.

I tend to set won't fix on #641 as the major problem was jupyterlab hanging because of a big repo. So with this PR and the virtual files list, I think if the UI is still hanging it should be investigated instead of hidden.

@fcollonval
Copy link
Member Author

Is it possible to immediately start a refresh when the git tab is brought back into view?

Done 😄

@ianhi
Copy link
Collaborator

ianhi commented Nov 1, 2020

Looks mostly good except I seem to be getting double status requests:

image

@ianhi
Copy link
Collaborator

ianhi commented Nov 1, 2020

Oh hmm. I see the same double status POST on master as well. Is that intentional or is that a bug?

schema/plugin.json Outdated Show resolved Hide resolved
Co-authored-by: Ian Hunt-Isaak <[email protected]>
@fcollonval
Copy link
Member Author

fcollonval commented Nov 3, 2020

Oh hmm. I see the same double status POST on master as well. Is that intentional or is that a bug?

Good catch, I'll take a look

Ok the double call comes from an emitted headChanged signal that triggers a refresh (in GitPanel.componentDidMount). The trigger is by

const headChanged = this._currentBranch !== data.current_branch;

Identity test on object...

@fcollonval
Copy link
Member Author

I replace the test by this one

      const headChanged =
        this._currentBranch.name !== data.current_branch.name ||
        this._currentBranch.top_commit !== data.current_branch.top_commit;

I think testing for the branch name and top commit get us cover.

model refresh -> emit signals -> update view
This renders the history tab coherent
@fcollonval fcollonval merged commit 120b04e into jupyterlab:master Nov 7, 2020
@fcollonval fcollonval deleted the fix/less-polling branch November 7, 2020 15:06
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