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

Update utils.py #225

Merged
merged 3 commits into from
Mar 12, 2023
Merged

Update utils.py #225

merged 3 commits into from
Mar 12, 2023

Conversation

hho6643
Copy link
Contributor

@hho6643 hho6643 commented Feb 11, 2023

verify get_meta isn't none before requesting db_table

Description

Patch to check that "query.get_meta()" is not None before getting db_table attribue.

Rationale

Using:
Django: 4.1.6
django-cachalot: 2.5.2
psycopg2: 2.9.5

During Model.validate_constraints() on new object save, I'm getting an "AttributeError" exception in _get_tables of utils.py at:
tables.add(query.get_meta().db_table); specifically when checking a FloatField constraint is greater than 0.

The issue is query.get_meta() is returning None, which doesn't have a db_table attribute. This patch checks for None before getting db_table.

The issue does not occur on Django 4.0.9.

verify get_meta isn't none before requesting db_table
@Andrew-Chen-Wang
Copy link
Collaborator

Can you add a test and fix CI?

@hho6643
Copy link
Contributor Author

hho6643 commented Feb 16, 2023

Can you add a test and fix CI?
Hi - Yes, I can add a test.

If I'm reading the CI issue correctly, though, it looks like errors keep getting generated because of a missing password for the DB. That seems like a separate problem unrelated to this patch? It's very possible I'm not reading this correctly.

psycopg2.OperationalError: fe_sendauth: no password supplied

@Andrew-Chen-Wang Andrew-Chen-Wang self-assigned this Mar 10, 2023
@Andrew-Chen-Wang
Copy link
Collaborator

@hho6643 I've fixed the CI errors. Would be great to get a test written here if your time permits. Thanks again!

@coveralls
Copy link

coveralls commented Mar 10, 2023

Pull Request Test Coverage Report for Build 4383434952

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.004%) to 97.202%

Totals Coverage Status
Change from base Build 2937621086: 0.004%
Covered Lines: 660
Relevant Lines: 679

💛 - Coveralls

@PetrDlouhy
Copy link
Contributor

PetrDlouhy commented Mar 10, 2023

@hho6643 I have written test that might be relvant here in #227, although there is one issue left to solve there.

@hho6643
Copy link
Contributor Author

hho6643 commented Mar 12, 2023

Hah, I finally got around today to start working on a test and it's already done!

@Andrew-Chen-Wang Andrew-Chen-Wang merged commit 866b662 into noripyt:master Mar 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants