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

Issue12/git clone timeout #48

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

andrewfulton9
Copy link
Collaborator

@andrewfulton9 andrewfulton9 commented Jan 21, 2025

Reference Issues or PRs

Closes #12

Adds a configurable timeout for git clone

What does this implement/fix?

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

Adds a test to ensure git clone timeout raises an error.

  • Did you test the pull request locally?
  • Did you add new tests?

Documentation

Access-centered content checklist

Text styling

  • The content is written with plain language (where relevant).
  • If there are headers, they use the proper header tags (with only one level-one header: H1 or # in markdown).
  • All links describe where they link to (for example, check the Nebari website).
  • This content adheres to the Nebari style guides.

Non-text content

  • All content is represented as text (for example, images need alt text, and videos need captions or descriptive transcripts).
  • If there are emojis, there are not more than three in a row.
  • Don't use flashing GIFs or videos.
  • If the content were to be read as plain text, it still makes sense, and no information is missing.

Any other comments?

I am not able to actually stop the thread running the git pull, so instead I just unblock the main thread on timeout. This means that the repo may still finish cloning even after the timeout. I tried to make it so that the status bar with the error message would go away if the repo is cloned despite the timeout, but it appears the fileChanged signal isn't triggered from the clone operation despite the the new folder being created. Is there another way to determine if the new folder is created? I think this may be related to #46. add the filesystem change isn't coming from within the filebrowser context

Copy link

Binder 👈 Launch a Binder on branch andrewfulton9/jupyterlab-gallery/issue12/git_clone_timeout

@andrewfulton9 andrewfulton9 added area: user experience 👩🏻‍💻 needs: review 👀 This PR is complete and ready for reviewing type: enhancement 💅🏼 New feature or request labels Jan 21, 2025
@andrewfulton9 andrewfulton9 added enhancement and removed type: enhancement 💅🏼 New feature or request labels Jan 22, 2025
@andrewfulton9 andrewfulton9 force-pushed the issue12/git_clone_timeout branch from 3fd9d1f to 7f898e8 Compare January 22, 2025 18:14
@andrewfulton9
Copy link
Collaborator Author

The odd-cases ui-test appears to be flaky. Sometimes it passes, sometimes the Private Repository image fails to load so I get this:

odd-cases-actual

I imagine that the tests will often have to be updated to account for new stats for things like stars, contributers ect. of the example github repos in some of the gallery items.

@andrewfulton9
Copy link
Collaborator Author

@krassowski, Could you give this a review when you get a chance?

jupyterlab_gallery/manager.py Outdated Show resolved Hide resolved
jupyterlab_gallery/manager.py Outdated Show resolved Hide resolved
andrewfulton9 and others added 2 commits January 27, 2025 15:40
Co-authored-by: Michał Krassowski <[email protected]>
Co-authored-by: Michał Krassowski <[email protected]>
@andrewfulton9 andrewfulton9 mentioned this pull request Jan 27, 2025
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a configurable timeout for clone
2 participants