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

Added Logs for get_os_path closes issue #1336

Merged
merged 5 commits into from
Oct 15, 2023

Conversation

jayeshsingh9767
Copy link
Contributor

closes issue jupyter/notebook#857

Changes made: added self.log.debug("Reading path from disk: %s", path) in filemanager.py in jupyter_server

@welcome
Copy link

welcome bot commented Oct 14, 2023

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@blink1073
Copy link
Contributor

Thanks @jayeshsingh9767! It looks like we could put the debug log in the _get_os_path and handle all of the cases you added.

@jayeshsingh9767
Copy link
Contributor Author

jayeshsingh9767 commented Oct 15, 2023

@blink1073 Thanks for the Quick Review, initially I too thought so but I was not sure if self.log object is available inside fileio.py will try out logging inside _get_os_path and check if all Testcases pass.

@blink1073
Copy link
Contributor

The typing failures are unrelated, I'll handle those in a follow up PR.

Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thank you!

@blink1073 blink1073 enabled auto-merge (squash) October 15, 2023 13:54
@blink1073
Copy link
Contributor

Oh actually, the lint is related, ha! I'll push the updates.

@jayeshsingh9767
Copy link
Contributor Author

Thanks @blink1073

@blink1073 blink1073 merged commit a984e07 into jupyter-server:main Oct 15, 2023
35 checks passed
@welcome
Copy link

welcome bot commented Oct 15, 2023

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

@Carreau
Copy link
Contributor

Carreau commented Oct 15, 2023

Congrats on closing a 7 years old issue ! 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants