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

Remove the compile() statement from regexp_match() #2606

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

nforro
Copy link
Member

@nforro nforro commented Oct 29, 2024

Related to #2596.

Fixes #2605.

@nforro nforro requested a review from a team as a code owner October 29, 2024 08:44
Copy link
Contributor

Copy link
Member

@lbarcziova lbarcziova left a comment

Choose a reason for hiding this comment

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

would it be hard to add a testcase for this? (in a followup PR would be ok as well)

Copy link
Member

@majamassarini majamassarini left a comment

Choose a reason for hiding this comment

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

Should we test it in tests_openshift/database? Or are we confident enough?

@nforro
Copy link
Member Author

nforro commented Oct 29, 2024

would it be hard to add a testcase for this? (in a followup PR would be ok as well)

I'm not sure, how would you go about testing this?

@nforro
Copy link
Member Author

nforro commented Oct 29, 2024

Should we test it in tests_openshift/database?

I thought we disabled those.

Or are we confident enough?

We are not 😅

@majamassarini
Copy link
Member

Should we test it in tests_openshift/database?

yes, but I am still running them locally. They are good for catching this kind of bugs. And you need just the postgres container running... but I can run them.

Signed-off-by: Nikola Forró <[email protected]>
@nforro
Copy link
Member Author

nforro commented Oct 29, 2024

I've added some tests and they pass locally, so it seems the fix works.

Copy link
Member

@lbarcziova lbarcziova left a comment

Choose a reason for hiding this comment

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

🙏

Copy link
Contributor

@nforro nforro added the mergeit When set, zuul wil gate and merge the PR. label Oct 29, 2024
Copy link
Contributor

Build succeeded (gate pipeline).
https://softwarefactory-project.io/zuul/t/packit-service/buildset/8d511b1964714a04b7da46718ac1a45b

✔️ pre-commit SUCCESS in 9m 38s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 82b48f8 into packit:main Oct 29, 2024
4 checks passed
@nforro nforro deleted the sidetags branch October 29, 2024 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit When set, zuul wil gate and merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to run the handler: SQL expression for WHERE/HAVING role expected, got <sqlalchemy.dialect...
3 participants