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

fixed merge conflicts and added verbose args #563

Merged
merged 3 commits into from
Jun 1, 2023

Conversation

TegveerG
Copy link

@TegveerG TegveerG commented May 31, 2023

Describe your changes

fixed merge conflicts by incorporating latest changes and add atexit line.
pytest run on 'src/tests/test_connection.py` successfully passes

Issue number

Closes #551
Closes #458

Checklist before requesting a review


📚 Documentation preview 📚: https://jupysql--563.org.readthedocs.build/en/563/

Copy link

@edublancas edublancas left a comment

Choose a reason for hiding this comment

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

I thought of an improvement, we could print a message so we let the user know we close the connections. essentially we can add a verbose (default should be false) argument to close_all, when true, we can print(f"Closing {key}")

then modify the atexit.register(close_all, verbose=True)

@TegveerG TegveerG requested a review from edublancas June 1, 2023 14:27
@TegveerG TegveerG changed the title fixed merge conflicts and added atextit: ready for review fixed merge conflicts and added verbose args Jun 1, 2023
@edublancas edublancas merged commit 05a1f36 into ploomber:master Jun 1, 2023
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.

close all connections when shutting down the kernel
2 participants