Skip to content

Revert PR 1426 - PostGIS param stripping. #1547

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

Merged
merged 4 commits into from
Dec 15, 2021
Merged

Revert PR 1426 - PostGIS param stripping. #1547

merged 4 commits into from
Dec 15, 2021

Conversation

tim-schilling
Copy link
Member

The stripping logic is stripping general sql parameters and the test
written for it does not run. I'm not confident that the test actually
hits the code we expect it to run either.

Closes #1543
Reopens #423

The stripping logic is stripping general sql parameters and the test
written for it does not run. I'm not confident that the test actually
hits the code we expect it to run either.

Closes #1543
Reopens #423
@codecov
Copy link

codecov bot commented Dec 15, 2021

Codecov Report

Merging #1547 (972aa14) into main (69506fb) will decrease coverage by 0.20%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1547      +/-   ##
==========================================
- Coverage   85.91%   85.71%   -0.21%     
==========================================
  Files          36       36              
  Lines        1889     1883       -6     
  Branches      313      310       -3     
==========================================
- Hits         1623     1614       -9     
- Misses        187      190       +3     
  Partials       79       79              
Impacted Files Coverage Δ
debug_toolbar/panels/sql/tracking.py 88.23% <ø> (-0.57%) ⬇️
debug_toolbar/panels/profiling.py 85.41% <0.00%> (-3.13%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69506fb...972aa14. Read the comment docs.

@tim-schilling
Copy link
Member Author

I kept the postgis testing so things are easier to manage in the future if we reattempt #423. Though I was close to removing it all.

@tim-schilling tim-schilling merged commit a5294e3 into django-commons:main Dec 15, 2021
@tim-schilling tim-schilling deleted the revert-1426-postgis-stripping branch December 15, 2021 20:34
@matthiask
Copy link
Member

Thanks! For the record, I definitely agree with this decision. I don't know what I was thinking when merging the PR, probably not too much :/

@tim-schilling
Copy link
Member Author

Eh, it happens. I wouldn't lose sleep over it. I'm not sure there's a great answer to preventing this in the future other than more involvement on our parts when assessing PRs.

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.

Some String Has Truncated Before Model Save In Releases Version==3.2.3
2 participants