Remove Vertica connector#26904
Conversation
|
FYI, https://trinodb.slack.com/archives/C07ABNN828M/p1755729147128999 is the relevant Slack thread. I'm waiting for a reply from OpenText. |
findepi
left a comment
There was a problem hiding this comment.
if we were to add this connector today, lack of testability options would be a blocker
for that reason, I consider this a rational move.
keeping it inline, but without ability to run tests, will make developing new features and new connector changes harder.
The connector may find new home outside of core trino repo, but still within trinodb org. It should be removed from here first, to define clear cut off version. Additionally, such a move should not be done, if there are no enough volunteers wishing to maintain the connector.
There was a problem hiding this comment.
There was significant effort involved to get this connector into Trino and OpenText was very interested in having the connector in Trino. They changed the license of the JDBC driver to allow this to proceed and had significant plans to enhance the connector and work on it. I think the change to make the container no longer available is probably not understanding the impact. We need to reach out and clarify this with their team and ensure they container becomes available again. If not they need to figure out how to proceed understanding that we can not continue to have the connector in the codebase..
Until we discussed this with them I think we should NOT remove the connector even if we just deactivate the tests for it for now. From what I know @ebyhr reached out. Please include myself and also potentially @martint @dain and @electrum in any communication with them so we can get some reasonable commitment from them to help on the work.
|
They might have had such enhancement plans, but in reality, there was no contribution as you can see here: https://github.com/trinodb/trino/pulls?q=vertica+is%3Apr+is%3Aclosed I sent an email to OpenText on Oct 10 (no response) and a reminder on Oct 21 (no response). |
|
I posted on Linkedin for the TSF and myself to see if we can get help from OpenText. Personally I would leave would vote for deactivating the test and leaving the connector in for a bit longer - at least until we hear back from them one way or another. |
Great, what's the timeline?
That would be a bad precedence. We do not accept connectors that we cannot test, because we do not ship stuff that might not work. |
We are waiting for OpenText to get back to us.
Sure.. but the connector is already here and we do not just throw them out without careful consideration either. There was significant work involved getting the connector in and removing it seems like a waste of time, especially since the vendor wants to collaborate. Maybe we have to find a means for community connectors outside the core, or even for now just deactivate the tests and exclude the connector from the shipped binaries. In either case .. I am against a hurried removal just so some vendor can then support it again on their own. Trino should be a community effort. |
f8c6438 to
7a2f8dc
Compare
7a2f8dc to
420e56c
Compare
|
This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack. |
420e56c to
6f42b1f
Compare
|
@mosabua i allowed myself to dismiss your 'request for changes' from 2 months ago. Clearly there was no sufficient follow-up. |
findepi
left a comment
There was a problem hiding this comment.
We recently had related problems on separate occasions
- OpenText removed Vertica images needed for testing the Vertica connector.
- MinIO removed MinIO images needed for testing the S3 filesystem with MinIO
- Exasol tests were very flaky and they had to be disabled
- Alluxio tests were considerably flaky.
What we did with in each of these cases
- we asked OpenText to help keep Vertica connector tested (thank you @mosabua!), but that wasn't enough
- we worked with Chainguard (thank you again @mosabua!) to switch to
cgr.dev/chainguard/minioMinIO images to keep our testing - we worked on Exasol images, versions and test scaffolding to resolve flakiness (thank you @pj-spoelders @ebyhr!). This wasn't enough. The process here is ongoing, so I think we're good here.
- we informed Alluxio company privately about the problem with @electrum nicely stating what should be obvious to all of us: "We can't maintain and ship code that is untested. Having reliable tests is a hard requirement.". Fortunately, the level of flakiness so far was bearable and didn't trigger a hard necessity to disable the tests for this integration.
Shipping Vertica connector with tests being disabled for quite some time would be inconsequential. It would be a bad precedence and would send a wrong signal into the community about double standards, about project policies that are supposed to be general, but are applied arbitrarily.
There was a problem hiding this comment.
Just to update on status. OpenText reached out again now to catch up and they are very interested to collaborate and keep the connector in. As such I think we should NOT merge this PR and instead meet with them as soon as possible and resolve the issue around testing with access to the relevant systems.
I will reach out to them to try and organize this as soon as possible. If really necessary we could deactivate the connector tests for now but we should NOT delete the whole code just to make work and having to add it back in.
So just like the Exasol connector and many other aspects .. we are working on improving this but it takes time.
We have reached agreement on the removal (see also https://trinodb.slack.com/archives/GKZ8GS0SK/p1767105323803829). We did not reach agreement on changing the agreement. Blocking PR is not warranted in this situation.
|
This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack. |
6f42b1f to
3da0f47
Compare
|
This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack. |
3da0f47 to
8019ec2
Compare
|
This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack. |
8019ec2 to
f704743
Compare
Description
I propose removing the Vertica connector since OpenText has removed all Docker images from Docker Hub.
We shouldn't release an untested connector.
vertica/vertica-containers#64 (comment) is a comment from OpenText:
Release notes