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.

Sign up for GitHub

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

Jump to bottom

*BREAKING CHANGE* Remove pyhdb #11469

Merged
merged 11 commits into from
Feb 14, 2022
Merged

*BREAKING CHANGE* Remove pyhdb #11469

merged 11 commits into from
Feb 14, 2022

Conversation

hithwen
Copy link
Contributor

@hithwen hithwen commented Feb 8, 2022 •
edited
Loading

pyhdb library has some stability issue so we cannnot, in good faith, leave users exposed to it. From 7.35 onwards users will need to perform a manual install of hdbcli to use SAP Hana integration.

This PR:

  • Removes pyhdb from the agent
  • Removes pyhdb compatibility from the integration
  • Updates documentation to inform the user that they need to manually install hdbcli to use the integration, while before it was an optional step

This PR is best viewed hiding whitespace changes

@hithwen hithwen changed the title Remove pyhdb *BREAKING CHANGE* Remove pyhdb Feb 8, 2022
@github-actions
Copy link

github-actions bot commented Feb 8, 2022

Label changelog/Changed was added to this Pull Request, so the next release will bump major version. Please make sure this is a breaking change, or use the changelog/Fixed label.

4 similar comments
@github-actions
Copy link

github-actions bot commented Feb 8, 2022

Label changelog/Changed was added to this Pull Request, so the next release will bump major version. Please make sure this is a breaking change, or use the changelog/Fixed label.

@github-actions
Copy link

github-actions bot commented Feb 8, 2022

Label changelog/Changed was added to this Pull Request, so the next release will bump major version. Please make sure this is a breaking change, or use the changelog/Fixed label.

@github-actions
Copy link

github-actions bot commented Feb 8, 2022

Label changelog/Changed was added to this Pull Request, so the next release will bump major version. Please make sure this is a breaking change, or use the changelog/Fixed label.

@github-actions
Copy link

github-actions bot commented Feb 8, 2022

Label changelog/Changed was added to this Pull Request, so the next release will bump major version. Please make sure this is a breaking change, or use the changelog/Fixed label.

@hithwen hithwen marked this pull request as ready for review February 10, 2022 12:19
@hithwen hithwen requested review from a team as code owners February 10, 2022 12:19
hithwen
hithwen commented Feb 10, 2022
View reviewed changes
sap_hana/tests/conftest.py
@@ -48,7 +48,7 @@ def initialize(self):
cursor.execute("BACKUP DATA USING FILE ('/tmp/backup')")

def connect(self):
self.conn = pyhdb.connect(**self.connection_args)
self.conn = HanaConnection(**self.connection_args)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may resolve the remaining flakiness on tests

alai97
alai97 previously approved these changes Feb 10, 2022
View reviewed changes
Copy link
Contributor

@alai97 alai97 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, four copy nits!

sap_hana/README.md Outdated Show resolved Hide resolved
sap_hana/README.md Outdated Show resolved Hide resolved
sap_hana/README.md Outdated Show resolved Hide resolved
sap_hana/tests/README.md Outdated Show resolved Hide resolved
@hithwen @alai97
Apply suggestions from code review
a856132 
Co-authored-by: Austin Lai <76412946+alai97@users.noreply.github.com>
@hithwen @alai97
Update sap_hana/tests/README.md
24c515b 
Co-authored-by: Austin Lai <76412946+alai97@users.noreply.github.com>
yzhan289
yzhan289 previously approved these changes Feb 11, 2022
View reviewed changes
@codecov
Copy link

codecov bot commented Feb 11, 2022 •
edited
Loading

Codecov Report

Merging #11469 (b35bda8) into master (ce3eaaa) will decrease coverage by 0.79%.
The diff coverage is 65.90%.

Flag Coverage Δ
sap_hana 91.06% <65.90%> (-1.34%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@sarah-witt
Copy link
Contributor

/azp run PR Changes

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@hithwen @sarah-witt
Update sap_hana/tests/common.py
b35bda8 
Co-authored-by: Sarah Witt <sarah.witt@datadoghq.com>
@hithwen hithwen merged commit 205201c into master Feb 14, 2022
@hithwen hithwen deleted the j/AI-2201-remove-shipped-library branch February 14, 2022 16:03
github-actions bot pushed a commit that referenced this pull request Feb 14, 2022
@hithwen @alai97
*BREAKING CHANGE* Remove pyhdb (#11469)
dcfdac6 
* Remove pyhdb

Co-authored-by: Austin Lai <76412946+alai97@users.noreply.github.com>
Co-authored-by: Sarah Witt <sarah.witt@datadoghq.com> 205201c
@yzhan289 yzhan289 mentioned this pull request Feb 16, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarah-witt sarah-witt sarah-witt approved these changes

@yzhan289 yzhan289 yzhan289 approved these changes

@ofek ofek ofek approved these changes

@sgnn7 sgnn7 sgnn7 approved these changes

@alai97 alai97 alai97 left review comments

Assignees
No one assigned
Labels
dependencies documentation integration/datadog_checks_base integration/sap_hana
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@hithwen @sarah-witt @sgnn7 @ofek @yzhan289 @alai97