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

Silence warnings display by python unit tests #3161

Merged

Conversation

portante
Copy link
Member

@portante portante commented Jan 12, 2023

There were two categories of warnings being displayed, those related to the xdist plugin, and those related to SQLAlchemy 2.0 migration.

The xdist plugin warnings are due to a bug in pytest-cov, see pytest-dev/pytest-cov#557. We just ignore them entirely for now, as that issue seems to be quiet.

The SQLAlchemy 2.0 migration warnings have 2 sub-categories. The first being that declarative_base has moved modules, and that warning is removed by using the proper import.

However, once that declarative_base warning is removed, one encounters:

.../pbench/server/database/models/datasets.py:875:
    RemovedIn20Warning: "Metadata" object is being merged into a
    Session along the backref cascade path for relationship
    "Dataset.metadatas"; in SQLAlchemy 2.0, this reverse cascade
    will not take place.  Set cascade_backrefs to False in either
    the relationship() or backref() function for the 2.0 behavior;
    or to set globally for the whole Session, set the future=True
    flag (Background on SQLAlchemy 2.0 at:
    https://sqlalche.me/e/b8d9)
        meta = Metadata(**kwargs)

Since we are no going to migrate to SQLAlchemy 2.0 any time soon, it seemed appropriate to just use the big hammer and turn the warnings off entirely.

dbutenhof
dbutenhof previously approved these changes Jan 12, 2023
Copy link
Member

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

Awesome!

Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

This is fine, but we should have a little bit more.

exec-tests Show resolved Hide resolved
pytest.ini Outdated Show resolved Hide resolved
dbutenhof
dbutenhof previously approved these changes Jan 12, 2023
webbnh
webbnh previously approved these changes Jan 12, 2023
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

Looks great...except.... 😁

exec-tests Show resolved Hide resolved
There were two categories of warnings being displayed, those related
to the xdist plugin, and those related to SQLAlchemy 2.0 migration.

The xdist plugin warnings are due to a bug in pytest-cov, see:
    pytest-dev/pytest-cov#557
We just ignore them entirely for now, as that issue seems to be quiet.

The SQLAlchemy 2.0 migration warnings have 2 sub-categories.  The
first being that `declarative_base` has moved modules, and that warning
is removed by using the proper import.

However, once that `declarative_base` warning is removed, one
encounters:

    .../pbench/server/database/models/datasets.py:875:
        RemovedIn20Warning: "Metadata" object is being merged into a
        Session along the backref cascade path for relationship
        "Dataset.metadatas"; in SQLAlchemy 2.0, this reverse cascade
        will not take place.  Set cascade_backrefs to False in either
        the relationship() or backref() function for the 2.0 behavior;
        or to set globally for the whole Session, set the future=True
        flag (Background on SQLAlchemy 2.0 at:
        https://sqlalche.me/e/b8d9)
            meta = Metadata(**kwargs)

Since we are no going to migrate to SQLAlchemy 2.0 any time soon, it
seemed appropriate to just use the big hammer and turn the warnings
off entirely.
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

👍

@portante portante merged commit b9de4ba into distributed-system-analysis:main Jan 12, 2023
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