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

Update docker_healthcheck.py #1882

Merged
merged 3 commits into from
Mar 3, 2023
Merged

Update docker_healthcheck.py #1882

merged 3 commits into from
Mar 3, 2023

Conversation

finalstate
Copy link
Contributor

@finalstate finalstate commented Mar 3, 2023

This does not (always) work. Sometimes, other JSON files appear in the directory, like this kernel-xxx file

jpserver-6.json
jpserver-6-open.html
kernel-374f4977-29fc-43e0-8a48-231f6980fdab.json

changing the glob expression to something like "jpserver*.json" instead of simply "*.json" should solve the problem (at least in my case, I checked with a quick patch on a live container, the healthcheck immediately turned 'healthy' again...)

Describe your changes

Issue ticket if applicable

Checklist (especially for first-time contributors)

  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests
  • I will try not to use force-push to make the review process easier for reviewers
  • I have updated the documentation for significant changes

This does not (always) work. Sometimes, other JSON files appear in the directory, like this kernel-xxx file

jpserver-6.json
jpserver-6-open.html
kernel-374f4977-29fc-43e0-8a48-231f6980fdab.json

changing the glob expression to something like "jpserver*.json" instead of simply "*.json" should solve the problem (at least in my case, I checked with a quick patch on a live container, the healthcheck immediately turned 'healthy' again...)
@mathbunnyru
Copy link
Member

Thank you for this PR @finalstate.
I will merge it when tests pass.

@mathbunnyru
Copy link
Member

When we use -e DOCKER_STACKS_JUPYTER_CMD=notebook, the file is called nbserver-7.json.

@finalstate
Copy link
Contributor Author

When we use -e DOCKER_STACKS_JUPYTER_CMD=notebook, the file is called nbserver-7.json.

'server-.json' ? this starts to become messy :-) Sorry, I am completely new to this project, so I am not of great help...

@mathbunnyru
Copy link
Member

It's gonna be *server-*.json.
This should work fine, I hope :)

It's nice that @imiro added so many test cases, so we can spot bugs like this.

@mathbunnyru mathbunnyru merged commit f16d1e3 into jupyter:main Mar 3, 2023
@finalstate finalstate deleted the patch-1 branch March 3, 2023 19:51
kentwait pushed a commit to kentwait/docker-stacks that referenced this pull request Aug 3, 2023
* Update docker_healthcheck.py

This does not (always) work. Sometimes, other JSON files appear in the directory, like this kernel-xxx file

jpserver-6.json
jpserver-6-open.html
kernel-374f4977-29fc-43e0-8a48-231f6980fdab.json

changing the glob expression to something like "jpserver*.json" instead of simply "*.json" should solve the problem (at least in my case, I checked with a quick patch on a live container, the healthcheck immediately turned 'healthy' again...)

* Update base-notebook/docker_healthcheck.py

* Update docker_healthcheck.py

---------

Co-authored-by: Ayaz Salikhov <mathbunnyru@users.noreply.github.com>
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