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

Skipping Cache Invalidations for Unsupported Raw Queries #248

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

git-yogeshkumar
Copy link

This is for issue #246
//: # (What're you proposing?)
I am raising a warning if the user is using Raw queries other than direct strings like psycopg2 Compose Objects for execution with cursor, I believe cachalot should not break if such cases are encountered, it should just not invalidate the cache if this happens and let the query execute.

Rationale

Since, raw queries can be executed using multiple formats, we should give users warning and don't raise unhandled exception as it causes unknown issues in running program.

@Andrew-Chen-Wang
Copy link
Collaborator

Andrew-Chen-Wang commented Oct 30, 2023

Thanks! Please add a test for this to catch the warning.

@coveralls
Copy link

coveralls commented Oct 30, 2023

Pull Request Test Coverage Report for Build 6688981804

  • 2 of 4 (50.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.3%) to 97.914%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cachalot/monkey_patch.py 2 4 50.0%
Totals Coverage Status
Change from base Build 6089987763: -0.3%
Covered Lines: 704
Relevant Lines: 719

💛 - Coveralls

@Andrew-Chen-Wang
Copy link
Collaborator

thank you!

@Andrew-Chen-Wang
Copy link
Collaborator

ci is failing

@git-yogeshkumar
Copy link
Author

is there any decorator that I can use to specify test case for a specifc DB only say postgres, cause in this case only postgres support pycopg objects other's don't!

@Andrew-Chen-Wang
Copy link
Collaborator

Hi I didn't notice that. You need to support all databases in this case, so I would just write it as plain SQL.

@scorpp
Copy link

scorpp commented Jan 29, 2024

@git-yogeshkumar I suggest you can use @skipUnless(connection.vendor == 'postgresql', ....) since I'm not aware of other db vendors supporting API like this.

see

@skipUnless(connection.vendor == 'postgresql',
'This test is only for PostgreSQL')
@override_settings(USE_TZ=True)
class PostgresReadTestCase(TestUtilsMixin, TransactionTestCase):

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.

4 participants