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

Make placeholder of DbApiHook configurable in UI #38528

Merged
merged 24 commits into from
Apr 6, 2024

Conversation

dabla
Copy link
Contributor

@dabla dabla commented Mar 27, 2024

Moved the placeholder property logic from OdbcHook to DbApiHook.
We now encountered the same issue when using jdbc connections as we did back then in odbc connections where the placeholder should be ? instead of %. As OdbcHook and JdbcHook both inherit DbApiHook, I moved the property there so both classes can use same logic. Also moved test of placeholder property from OdbcHook to DbApiHook and did some refactoring so both tests can reuse same mocking function.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

…ApiHook class so that the same logic can also be used with the JdbcHook
@Taragolis Taragolis added the full tests needed We need to run full set of tests for this PR to merge label Mar 27, 2024
@dabla dabla requested a review from Taragolis March 30, 2024 10:14
@eladkal eladkal changed the title Make "placeholder" of DbApiHook configurable in UI Make placeholder of DbApiHook configurable in UI Apr 4, 2024
@eladkal eladkal requested a review from potiuk April 4, 2024 08:33
@eladkal eladkal merged commit 0b1308c into apache:main Apr 6, 2024
68 checks passed
@dabla
Copy link
Contributor Author

dabla commented Apr 6, 2024

Thank you for merging this pull request, could someone have a look to check this one also please: #38707

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers full tests needed We need to run full set of tests for this PR to merge provider:common-sql provider:odbc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants