-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
feat: change query predicate to text #16160
feat: change query predicate to text #16160
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16160 +/- ##
==========================================
- Coverage 76.82% 76.55% -0.27%
==========================================
Files 996 996
Lines 52940 52940
Branches 6732 6732
==========================================
- Hits 40670 40529 -141
- Misses 12045 12186 +141
Partials 225 225
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the fix!
❗ Please consider rebasing your branch to avoid db migration conflicts. |
51eaf08
to
25e9041
Compare
.pylintrc
Outdated
@@ -251,7 +251,7 @@ expected-line-ending-format= | |||
# Logging modules to check that the string format arguments are in logging | |||
# function parameter format | |||
logging-modules=logging | |||
|
|||
disable=logging-fstring-interpolation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
disable=logging-fstring-interpolation |
Using f-strings with logging is not considered safe because if the string interpolation fails nothing gets logged. if you use the old-school string interpolation then the message always get logged, even if the interpolation fails.
for row in rows: | ||
row.fetch_values_predicate = None | ||
|
||
logging.info(f"{len(rows)} values deleted") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logging.info(f"{len(rows)} values deleted") | |
logging.info("%d values deleted", len(rows)) |
6677923
to
c4f44ca
Compare
c4f44ca
to
3ab46fb
Compare
* change query predicate to text * make input multiline * remove value that is too long for the downgrade * keep logging lint rule (cherry picked from commit 628169a)
* change query predicate to text * make input multiline * remove value that is too long for the downgrade * keep logging lint rule (cherry picked from commit 628169a)
* change query predicate to text * make input multiline * remove value that is too long for the downgrade * keep logging lint rule
* change query predicate to text * make input multiline * remove value that is too long for the downgrade * keep logging lint rule (cherry picked from commit 628169a)
* change query predicate to text * make input multiline * remove value that is too long for the downgrade * keep logging lint rule
* change query predicate to text * make input multiline * remove value that is too long for the downgrade * keep logging lint rule (cherry picked from commit 628169a)
* change query predicate to text * make input multiline * remove value that is too long for the downgrade * keep logging lint rule (cherry picked from commit c345885)
SUMMARY
changes query predicate to text to accommodate longer strings
Closes #16100
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Results from migration script:
Current: 0.34 s
10+: 0.22 s
100+: 0.23 s
1000+: 0.37 s
ADDITIONAL INFORMATION