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

Documentation for PostgreSQL integration testing #663

Closed
wants to merge 12 commits into from

Conversation

ha2179
Copy link

@ha2179 ha2179 commented Jun 25, 2023

Describe your changes

Added postgreSQL Test Documentation ipynb file; opening draft PR

Issue number

Closes 643

Checklist before requesting a review


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

@ha2179
Copy link
Author

ha2179 commented Jun 25, 2023

Hello @edublancas
I started with setting up development environment for jupysql as directed in the documentation. After that I started PostgreSQL instance up and running (docker container) and ran the tests in both the files mentioned in issue #643. As everything is working fine till now, I am starting with the proper documentation in separate notebook file. Just wanted to check if am I on the right track?

I will be pushing the initial draft of a file shortly.

@ha2179 ha2179 force-pushed the 643-document-postgres-test branch from 0a8eeeb to 22cb1a3 Compare June 26, 2023 05:47
@ha2179
Copy link
Author

ha2179 commented Jun 26, 2023

@edublancas I have pushed the initial draft of test document to the repo. Should I make it available for review?

@ha2179 ha2179 marked this pull request as ready for review June 27, 2023 02:23
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.

did you run the commands you added to the tutorial? I believe dockerctx depends on psycopg2, so psycopg2 should be installed before dockerctx.

Also, I don't think %pytest will work.

It'd be better to run the commands to ensure they actually work: https://ploomber-contributing.readthedocs.io/en/latest/documentation/notebooks.html

also, the docs are failing. check the logs and fix the warnings.

@neelasha23
Copy link

Not able to see the docs preview. Please fix the build and re-request for review
@ha2179

@ha2179
Copy link
Author

ha2179 commented Jun 27, 2023

did you run the commands you added to the tutorial? I believe dockerctx depends on psycopg2, so psycopg2 should be installed before dockerctx.

Also, I don't think %pytest will work.

It'd be better to run the commands to ensure they actually work: https://ploomber-contributing.readthedocs.io/en/latest/documentation/notebooks.html

also, the docs are failing. check the logs and fix the warnings.

Sure, will do that. Should I make this a markdown file and move it under tutorials/ ?

@edublancas
Copy link

Sure, will do that. Should I make this a markdown file and move it under tutorials/ ?

No. keep it as an ipynb. add if in the same section as the one documenting how to run integration tests

@ha2179
Copy link
Author

ha2179 commented Jun 27, 2023

updated the doc as per requested changes, and fixed the build warnings. now docs is up.

@ha2179 ha2179 changed the title created postgres-test-doc Documentation for PostgreSQL integration testing Jun 27, 2023
@ha2179 ha2179 requested a review from yafimvo as a code owner June 28, 2023 03:06
@ha2179
Copy link
Author

ha2179 commented Jun 28, 2023

apologies for accidentally tagging @yafimvo for review due to yaml change, reverted the change already

…gres-test

merging with master to update ci.yml fix
@ha2179 ha2179 requested a review from edublancas June 28, 2023 03:48
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.

looks like you're missing a changelog entry. please add

also, if you look at the integration tests implementation, you'll see that it's unnecessary to run the docker commands, as the test will spin up the container when it starts.

another thing I noticed when running your commands is that pytest will take ~1 minute to fail if I'm missing psycopg2, looks like we have an issue in the setup because missing a package should exit the command fairly quickly with an error message. so please investigate what's going on

@ha2179
Copy link
Author

ha2179 commented Jun 28, 2023

looks like you're missing a changelog entry. please add

also, if you look at the integration tests implementation, you'll see that it's unnecessary to run the docker commands, as the test will spin up the container when it starts.

another thing I noticed when running your commands is that pytest will take ~1 minute to fail if I'm missing psycopg2, looks like we have an issue in the setup because missing a package should exit the command fairly quickly with an error message. so please investigate what's going on

Yes that's right, removed the docker run part from doc and updated the changelog.
Sure, will investigate over that issue!

@ha2179
Copy link
Author

ha2179 commented Jun 28, 2023

looks like you're missing a changelog entry. please add

also, if you look at the integration tests implementation, you'll see that it's unnecessary to run the docker commands, as the test will spin up the container when it starts.

another thing I noticed when running your commands is that pytest will take ~1 minute to fail if I'm missing psycopg2, looks like we have an issue in the setup because missing a package should exit the command fairly quickly with an error message. so please investigate what's going on

Hello, can't seem to reproduce this issue on my end. I tried setting up entirely new environment, where it doesn't let me install dockerctx without psycopg2, hence pytest instantly fails over dockerctx.
Also tried with deliberately uninstalling psycopg2 and running pytest - in this case it produces expected error, and Fails all tests almost fairly quickly under ~15 seconds for me.

What's exactly happening on your end?

Screenshot 2023-06-28 at 3 06 27 PM

@edublancas
Copy link

@ha2179 looks like yours is failing fast because you explicitly added import psycopg2?

this would imply that we'd have to add an import {package} to each database we have in the integration tests, instead, we should find the root issue that's causing this to hang up. This is what happens in my environment (note that it takes 1 minute):

pytest "src/tests/integration/test_generic_db_operations.py" -k "ip_with_postgreSQL"
FAILED src/tests/integration/test_generic_db_operations.py::test_query_count[ip_with_postgreSQL-3] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_create_table_with_indexed_df[ip_with_postgreSQL-15-15] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_active_connection_number[ip_with_postgreSQL-1] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_close_and_connect[ip_with_postgreSQL-postgreSQL] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_telemetry_execute_command_has_connection_info[ip_with_postgreSQL-postgresql-psycopg2] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_histogram[ip_with_postgreSQL-histogram] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_histogram[ip_with_postgreSQL-hist] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_histogram[ip_with_postgreSQL-histogram-bins] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_boxplot[ip_with_postgreSQL-boxplot] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_boxplot[ip_with_postgreSQL-box] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_boxplot[ip_with_postgreSQL-boxplot-with-horizontal] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_boxplot[ip_with_postgreSQL-boxplot-with] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sql_cmd_magic_uno[ip_with_postgreSQL] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sql_cmd_magic_dos[ip_with_postgreSQL] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_profile_data_mismatch[ip_with_postgreSQL] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlcmd_tables_columns[ip_with_postgreSQL-numbers] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlcmd_tables[ip_with_postgreSQL] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sql_query[ip_with_postgreSQL-simple-query] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sql_query[ip_with_postgreSQL-cte] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sql_query[ip_with_postgreSQL-interpolation-like-comment] - ModuleNotFoundError: No module named 'psycopg2'
============================================================= 20 failed, 1 skipped, 130 deselected in 62.89s (0:01:02) ==============================================================

@ha2179
Copy link
Author

ha2179 commented Jun 28, 2023

@ha2179 looks like yours is failing fast because you explicitly added import psycopg2?

this would imply that we'd have to add an import {package} to each database we have in the integration tests, instead, we should find the root issue that's causing this to hang up. This is what happens in my environment (note that it takes 1 minute):

pytest "src/tests/integration/test_generic_db_operations.py" -k "ip_with_postgreSQL"
FAILED src/tests/integration/test_generic_db_operations.py::test_query_count[ip_with_postgreSQL-3] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_create_table_with_indexed_df[ip_with_postgreSQL-15-15] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_active_connection_number[ip_with_postgreSQL-1] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_close_and_connect[ip_with_postgreSQL-postgreSQL] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_telemetry_execute_command_has_connection_info[ip_with_postgreSQL-postgresql-psycopg2] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_histogram[ip_with_postgreSQL-histogram] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_histogram[ip_with_postgreSQL-hist] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_histogram[ip_with_postgreSQL-histogram-bins] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_boxplot[ip_with_postgreSQL-boxplot] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_boxplot[ip_with_postgreSQL-box] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_boxplot[ip_with_postgreSQL-boxplot-with-horizontal] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlplot_boxplot[ip_with_postgreSQL-boxplot-with] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sql_cmd_magic_uno[ip_with_postgreSQL] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sql_cmd_magic_dos[ip_with_postgreSQL] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_profile_data_mismatch[ip_with_postgreSQL] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlcmd_tables_columns[ip_with_postgreSQL-numbers] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sqlcmd_tables[ip_with_postgreSQL] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sql_query[ip_with_postgreSQL-simple-query] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sql_query[ip_with_postgreSQL-cte] - ModuleNotFoundError: No module named 'psycopg2'
FAILED src/tests/integration/test_generic_db_operations.py::test_sql_query[ip_with_postgreSQL-interpolation-like-comment] - ModuleNotFoundError: No module named 'psycopg2'
============================================================= 20 failed, 1 skipped, 130 deselected in 62.89s (0:01:02) ==============================================================

Nope, I didn't edit anything. The import psycopg2 we are seeing is function call to sqlalchemy's method for postgreSQL

Here's how complete traceback looks on my side -

src/tests/integration/test_generic_db_operations.py:51: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/_pytest/fixtures.py:585: in getfixturevalue
    fixturedef = self._get_active_fixturedef(argname)
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/_pytest/fixtures.py:607: in _get_active_fixturedef
    self._compute_fixture_value(fixturedef)
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/_pytest/fixtures.py:693: in _compute_fixture_value
    fixturedef.execute(request=subrequest)
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/_pytest/fixtures.py:1045: in execute
    fixturedef = request._get_active_fixturedef(argname)
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/_pytest/fixtures.py:607: in _get_active_fixturedef
    self._compute_fixture_value(fixturedef)
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/_pytest/fixtures.py:693: in _compute_fixture_value
    fixturedef.execute(request=subrequest)
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/_pytest/fixtures.py:1069: in execute
    result = ihook.pytest_fixture_setup(fixturedef=self, request=request)
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/pluggy/_hooks.py:433: in __call__
    return self._hookexec(self.name, self._hookimpls, kwargs, firstresult)
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/pluggy/_manager.py:112: in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/_pytest/fixtures.py:1123: in pytest_fixture_setup
    result = call_fixture_func(fixturefunc, request, kwargs)
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/_pytest/fixtures.py:895: in call_fixture_func
    fixture_result = next(generator)
src/tests/integration/conftest.py:102: in setup_postgreSQL
    engine = create_engine(
<string>:2: in create_engine
    ???
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/sqlalchemy/util/deprecations.py:281: in warned
    return fn(*args, **kwargs)  # type: ignore[no-any-return]
../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/sqlalchemy/engine/create.py:601: in create_engine
    dbapi = dbapi_meth(**dbapi_args)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

cls = <class 'sqlalchemy.dialects.postgresql.psycopg2.PGDialect_psycopg2'>

    @classmethod
    def import_dbapi(cls):
>       import psycopg2
E       ModuleNotFoundError: No module named 'psycopg2'

../../opt/anaconda3/envs/jupysql/lib/python3.10/site-packages/sqlalchemy/dialects/postgresql/psycopg2.py:669: ModuleNotFoundError

Really surprised why it's taking so much longer to fail on your side despite failing with exact same error.

@edublancas
Copy link

edublancas commented Jun 28, 2023

are you starting the container manually? because I'm not (the tests are configured to start one if needed). I think the overhead might come from that

@ha2179
Copy link
Author

ha2179 commented Jun 28, 2023

are you starting the container manually? because I'm not (the tests are configured to start one if needed). I think the overhead might come from that

Spotted it. Apparently there was stopped container mapped to the port, maybe that was expediting it. Now, it's taking ~1 min for me as well. Will track down what's happening!

@ha2179
Copy link
Author

ha2179 commented Jun 29, 2023

are you starting the container manually? because I'm not (the tests are configured to start one if needed). I think the overhead might come from that

Spotted it. Apparently there was stopped container mapped to the port, maybe that was expediting it. Now, it's taking ~1 min for me as well. Will track down what's happening!

@edublancas found the root cause -
It's basically a timeout issue. In jupysql/src/sql/_testing.py this function checks if the database connection is ready, and has timeout set at 60 that's why it was taking 1 min to fail; I tried playing around with timeout variable, after setting it to 1, the tests instantly fail over the ModuleNotFoundError.

Should we change this to some lower amount like ~10 secs?

def database_ready(
    database,
    timeout=60,
    poll_freq=0.5,
):
    """Wait until the container is ready to receive connections.


    :type host: str
    :type port: int
    :type timeout: float
    :type poll_freq: float
    """

fixed postgres test timeout issue
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.

3 participants