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

Warnings during ORM creation due to SQLModel metaclass persistence #48

Closed
donaldcampbelljr opened this issue May 24, 2023 · 4 comments · Fixed by #142
Closed

Warnings during ORM creation due to SQLModel metaclass persistence #48

donaldcampbelljr opened this issue May 24, 2023 · 4 comments · Fixed by #142

Comments

@donaldcampbelljr
Copy link
Contributor

Encountered during pytesting:

tests/test_pipestat.py: 43 warnings
  /home/drc/GITHUB/pipestat/pipestat/venv/lib/python3.10/site-packages/sqlmodel/main.py:369: SAWarning: This declarative base already contains a class with the same class name and module name as pydantic.main.test_pipe__project, and will be replaced in the string-lookup table.
    DeclarativeMeta.__init__(cls, classname, bases, dict_used, **kw)

tests/test_pipestat.py: 41 warnings
  /home/drc/GITHUB/pipestat/pipestat/venv/lib/python3.10/site-packages/sqlmodel/main.py:369: SAWarning: This declarative base already contains a class with the same class name and module name as pydantic.main.test_pipe__sample, and will be replaced in the string-lookup table.
    DeclarativeMeta.__init__(cls, classname, bases, dict_used, **kw)

This appears to stem from the fact that the SQLModel metaclass is persistent during our pytest testing and contains the tablenames from the previous pytest:
image

In the next test, we create ORMs with the same names, so we get a warning that they already exist.

with ContextManagerDBTesting(db_url) as connection:
    psm = pipestat.PipestatManager(
        schema_path="sample_output_schema.yaml",
        #namespace="test",
        record_identifier="constant_record_id",
        database_only=True,
        config="drcdbconfig.yaml",
    )
    psm.report(values=val[0], strict_type=False, project_level=False)


with ContextManagerDBTesting(db_url) as connection:
    psm2 = pipestat.PipestatManager(
        schema_path="sample_output_schema.yaml",
        #namespace="test",
        record_identifier="constant_record_id",
        database_only=True,
        config="drcdbconfig.yaml",
    )
    val = [{"name_of_something": "test_name"},
           {"number_of_things": 1},
           {"percentage_of_things": 10.1}]
    psm2.report(values=val[0], strict_type=False, project_level=False)

In the above example, the warning will fire when psm2 is created since SQLModel still contains the tablenames from the creation of psm.

We see this warning fire over and over during pytesting because we create many new pipestatmanger objects during testing that all end up sharing the SQLModel metaclass.

Originally posted by @donaldcampbelljr in #26 (comment)

@donaldcampbelljr
Copy link
Contributor Author

Persisting here:
pipestat.SQLModel.metadata

@donaldcampbelljr
Copy link
Contributor Author

Discussion on 20 May 2023:
-This is probably a benign issue and we should attempt to reload the metaclass registry between executing pytests (via context manager?) so that we no longer get this warning.

@donaldcampbelljr donaldcampbelljr added this to the v0.4.0 milestone May 30, 2023
donaldcampbelljr added a commit that referenced this issue Jun 16, 2023
@donaldcampbelljr
Copy link
Contributor Author

Partial fix implemented where:
from sqlmodel.main import default_registry

class ContextManagerDBTesting:
    """
    Creates context manager to connect to database at db_url and drop everything from the database upon exit to ensure
    the db is empty for each new test.
    """

    def __init__(self, db_url):
        self.db_url = db_url

    def __enter__(self):
        self.engine = create_engine(self.db_url, echo=True)
        self.connection = self.engine.connect()
        return self.connection

    def __exit__(self, exc_type, exc_value, exc_traceback):
        SQLModel.metadata.drop_all(self.engine)
        default_registry.dispose()
        self.connection.close()

If default_registry is disposed of between tests, this clears the persistent data and the warning no longer fires. Unfortunately, this appears to only work in the .py file where the context manager is declared, i.e. the issue is solved within test_db_mode_only.py but it is still happening in test_pipestat.py even though we import the context manager into that file.

@nsheff nsheff removed this from the v0.4.0 milestone Jun 20, 2023
@donaldcampbelljr donaldcampbelljr added this to the 0.8.0 milestone Jan 17, 2024
@donaldcampbelljr
Copy link
Contributor Author

Ok, this is now solved. Looks like it was only one remaining test that was producing these. Placing it within a context manager solved the 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 a pull request may close this issue.

2 participants