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

feat(inputs.sql): Add support for IBM Netezza #14200

Merged
merged 5 commits into from
Nov 16, 2023
Merged

Conversation

mplpl
Copy link
Contributor

@mplpl mplpl commented Oct 27, 2023

Required for all PRs

Added support for IBM Netezza data warehouse to inputs/sql plugin.

@telegraf-tiger
Copy link
Contributor

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

@telegraf-tiger telegraf-tiger bot added area/sql feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Oct 27, 2023
Copy link
Contributor

@powersj powersj left a comment

Choose a reason for hiding this comment

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

Hi,

Thanks for the PR. One small comment below, however the 32-bit build failures are the larger concern. I see you commented on IBM/nzgo#38

go.mod Outdated Show resolved Hide resolved
@powersj powersj self-assigned this Oct 27, 2023
@powersj
Copy link
Contributor

powersj commented Oct 27, 2023

What is your plan to resolve the 32-bit build issues?

@powersj powersj added the waiting for response waiting for response from contributor label Oct 27, 2023
@mplpl
Copy link
Contributor Author

mplpl commented Oct 27, 2023

I'm going to talk to IBM on Monday about that. It may take some time for this issue to become fixed.

@telegraf-tiger telegraf-tiger bot removed the waiting for response waiting for response from contributor label Oct 27, 2023
@srebhan
Copy link
Member

srebhan commented Oct 31, 2023

Just as a hint: As workaround you could guard the driver with build-tags excluding 32-bit archs (similar to sqlite)...

@mplpl
Copy link
Contributor Author

mplpl commented Oct 31, 2023

Thanks for suggestion @srebhan
In the meanwhile I've opened a PR to nzgo to support 32-bit environments. Let's see how it goes (I'm in touch with the nzgo maintainer). If it does not, I'll go sqlite way.

@powersj powersj added the waiting for response waiting for response from contributor label Nov 3, 2023
@powersj
Copy link
Contributor

powersj commented Nov 10, 2023

@mplpl,

Looks like IBM/nzgo#60 was landed. Is an update to the version imported all that is required?

@telegraf-tiger telegraf-tiger bot removed the waiting for response waiting for response from contributor label Nov 10, 2023
@powersj powersj added the waiting for response waiting for response from contributor label Nov 10, 2023
go.mod Outdated Show resolved Hide resolved
@mplpl
Copy link
Contributor Author

mplpl commented Nov 13, 2023

@powersj My PR with support for 32-bit envs for nzgo was indeed merged, but a new version of this driver has not yet been released. I'm in touch with the maintainers and waiting for their decision about when to release it.

@telegraf-tiger telegraf-tiger bot removed the waiting for response waiting for response from contributor label Nov 13, 2023
@powersj
Copy link
Contributor

powersj commented Nov 13, 2023

github.com/IBM/nzgo

They haven't seem to have tagged a release in 2 years. Our next feature release is in a few weeks and I would like to see this land in that, instead of waiting till March.

What we can do is import the repo at a specific commit, rather than a specific tag/release. Then if/when they do an actual release we can switch the go.mod entry to a commit hash.

@powersj
Copy link
Contributor

powersj commented Nov 15, 2023

@mplpl I have checked out your branch, rebased on our master, updated it to use nzgo with the master branch from today, and run go mod tidy.

@mplpl
Copy link
Contributor Author

mplpl commented Nov 15, 2023

@powersj ok, thanks. It is Diwali holiday in India, and the person responsible for nzgo is away now. But she should be back in a few days and promised to update me with when they plan to tag a new version.

@telegraf-tiger
Copy link
Contributor

Copy link
Contributor

@Hipska Hipska left a comment

Choose a reason for hiding this comment

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

Is it possible to add integration test like we have for others? Or would that not be easy to achieve?

@Hipska Hipska added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Nov 16, 2023
@srebhan
Copy link
Member

srebhan commented Nov 16, 2023

@Hipska I don't see any official container image, so having an integration test would be hard...

Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @mplpl! Feel free to bump the driver once a new release is out.

@srebhan srebhan merged commit a4f8b45 into influxdata:master Nov 16, 2023
23 checks passed
@github-actions github-actions bot added this to the v1.29.0 milestone Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sql feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants