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

performance issue when checking version #70

Open
edublancas opened this issue Jul 14, 2023 · 2 comments
Open

performance issue when checking version #70

edublancas opened this issue Jul 14, 2023 · 2 comments

Comments

@edublancas
Copy link
Contributor

edublancas commented Jul 14, 2023

in ploomber/jupysql#734, I noticed that JupySQL's unit tests were taking too long to run, after running some profiling I realized that this was because of the logic in this package that checks whether there are new updates:

def check_version(package_name, version):

Here's what was happening (10 seconds to run a test!)

(jupysql) eduardo@macbookair dev/jupysql (ci-speed) » pytest src/tests/test_magic.py::test_memory_db
================================================================================ test session starts ================================================================================
platform darwin -- Python 3.10.12, pytest-7.4.0, pluggy-1.2.0
rootdir: /Users/eduardo/dev/jupysql
configfile: pyproject.toml
plugins: anyio-3.7.1, profiling-1.7.0
collected 1 item

src/tests/test_magic.py .                                                                                                                                                     [100%]

================================================================================= 1 passed in 9.86s =================================================================================

After disabling the version check via the env variable (<1 second!):

(jupysql) eduardo@macbookair dev/jupysql (ci-speed) » export PLOOMBER_VERSION_CHECK_DISABLED=true                                                                                2 ↵
(jupysql) eduardo@macbookair dev/jupysql (ci-speed) » pytest src/tests/test_magic.py::test_memory_db
================================================================================ test session starts ================================================================================
platform darwin -- Python 3.10.12, pytest-7.4.0, pluggy-1.2.0
rootdir: /Users/eduardo/dev/jupysql
configfile: pyproject.toml
plugins: anyio-3.7.1, profiling-1.7.0
collected 1 item

src/tests/test_magic.py .                                                                                                                                                     [100%]

================================================================================= 1 passed in 0.26s =================================================================================

JupySQL's unit tests have been fixed already by setting the environment variable. However, we need to get to the root cause of this because the version check if performed in all our packages, might impact performance. So we have to understand why the version check was delaying the tests so much and if this is affecting the user experience.

tools we can use for profiling:

https://github.com/man-group/pytest-plugins/blob/master/pytest-profiling/README.md
https://github.com/pyutils/line_profiler

@edublancas edublancas added the stash Label used to categorize issues that will be worked on next label Jul 14, 2023
@tonykploomber
Copy link
Contributor

AC:
To find the bottleneck of why check_version took too long to run

@edublancas
Copy link
Contributor Author

sounds good!

@edublancas edublancas removed the stash Label used to categorize issues that will be worked on next label Jan 9, 2024
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

No branches or pull requests

2 participants