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.

Sign up for GitHub

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

Jump to bottom

Workaround shutdown issue with aiosqlite for now #278

Merged
merged 3 commits into from
Apr 2, 2024
Merged

Workaround shutdown issue with aiosqlite for now #278

merged 3 commits into from
Apr 2, 2024

Conversation

krassowski
Copy link
Member

Addresses issue discussed in #252 to help bring the CI green again.

The Windows tests are still failing though - are these indicative or Windows-specific bugs?

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 1, 2024

Binder 👈 Launch a Binder on branch krassowski/jupyter_collaboration/workaround-test-failure

@davidbrochart
Copy link
Collaborator

From the macOS failure it looks like your workaround could also be applied to test_room_concurrent_initialization?
The Windows failure is caused by the jupyter_server_fileid manager which does some cleanup on garbage collection, which I don't think is a good practice.

@krassowski
Copy link
Member Author

Well, down to one failure:

image

I guess we can try to force jupyter_server_fileid to delete during the test too?

@davidbrochart
Copy link
Collaborator

But the failure means that your workaround didn't work, right?

@krassowski
Copy link
Member Author

Not quite sure. Could it have been an interaction with jupyter_server_fileid? It is all passing now?

@davidbrochart
Copy link
Collaborator

I think it's flaky, we should rerun tests to check.

@krassowski
Copy link
Member Author

I will close and reopen to see if we get all green again (of note Tests / Test Minimum Versions (pull_request) looks stuck right now).

@krassowski krassowski closed this Apr 2, 2024
@krassowski krassowski reopened this Apr 2, 2024
davidbrochart
davidbrochart approved these changes Apr 2, 2024
View reviewed changes
Copy link
Collaborator

@davidbrochart davidbrochart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reran tests 2 more times, your workaround seems to work fine.
Now it's test_dirty that seems to be flaky 😄

@krassowski krassowski merged commit b49bc4f into jupyterlab:main Apr 2, 2024
36 of 38 checks passed
krassowski added a commit to krassowski/jupyter_collaboration that referenced this pull request Apr 11, 2024
@krassowski
Workaround shutdown issue with aiosqlite for now (jupyterlab#278)
489004a 
* Workaround shutdown issue with `aiosqlite` for now

* Also wait to stop in `test_room_concurrent_initialization`

* Try to delete jupyter_server_fileid early
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbrochart davidbrochart davidbrochart approved these changes

Assignees
No one assigned
Labels
maintenance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@krassowski @davidbrochart