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

Opensearch operator set connection type #39788

Merged
merged 12 commits into from
Jun 8, 2024

Conversation

Lukas1v
Copy link
Contributor

@Lukas1v Lukas1v commented May 23, 2024


We ran into issues when trying the Opensearch operator to connect to an endpoint with a self-signed certificate. It even failed with the following connection settings:

{
  "use_ssl": "True",
  "verify_certs": "False"
}

Those parameters were converted to strings and not as booleans.

Another issue we faced is that the connection type RequestsHttpConnection did not work for us. We discovered it because we knew the Opensearch client itself worked which uses by default a Urllib3HttpConnection, but the hook does not.

This PR addresses both issues:

  • The parameters use_ssl and verify_certs are both converted to boolean in the hook
  • an option is provided to specify an existing Opensearch connection type. If nothing is provided, RequestsHttpConnection is used as the default

^ 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.

@Taragolis
Copy link
Contributor

It even failed with the following connection settings:

{
"use_ssl": "True",
"verify_certs": "False"
}

{
    "use_ssl": true, 
    "verify_certs": false
}

@Lukas1v Lukas1v requested a review from Taragolis May 28, 2024 14:59
Copy link
Contributor

@cjames23 cjames23 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@eladkal eladkal merged commit 1a61eb3 into apache:main Jun 8, 2024
49 checks passed
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
…rtificate (apache#39788)

* feat: added connection options

* feat: opensearch hook unit tests

* feat: fallback to RequestsHttpConnection

* fix: static checks

* fix: static checks

* fix: static checks

* feat: opensearch static module loading

---------

Co-authored-by: Lukas Verret <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants