-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Test Hive S3 datalake over proxy #11310
Test Hive S3 datalake over proxy #11310
Conversation
16f902e
to
6af27ce
Compare
...ng/trino-testing-containers/src/main/java/io/trino/testing/containers/BaseTestContainer.java
Outdated
Show resolved
Hide resolved
plugin/trino-hive/src/test/java/io/trino/plugin/hive/containers/HiveMinioDataLake.java
Outdated
Show resolved
Hide resolved
plugin/trino-hive/src/test/java/io/trino/plugin/hive/containers/HiveMinioDataLake.java
Outdated
Show resolved
Hide resolved
testing/trino-testing-containers/src/main/java/io/trino/testing/containers/Httpd.java
Show resolved
Hide resolved
public class TestHive2OnDataLakeOverProxy | ||
extends BaseTestHiveOnDataLake |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How many, and which tests do we run with S3 HTTP proxy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the first one in trino repository. And in this particular case all underlining tests in class will be using S3 (MinIO) over proxy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, but the tests actually invoked look like a random selection. For example flush procedure tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that's true. Cause behind my decision is that. I needed:
- Hive Data Lake set up
- test's utilising complex operations on S3 (MinIO).
BaseTestHiveOnDataLake test seems to be perfect fit aside cache flush (which are pretty fast so shouldn't be a big deal).
We could ofcourse sub-select some tests and either replicate them or make separate base class.
But I'm not sure if it's worth. In current setup it's a natural place to add new use cases for hive data lake. And we will have a proof that all of those use cases would work over proxy as well (which i agree might be an overkill 🤷♂️ )
plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHive2OnDataLakeOverProxy.java
Outdated
Show resolved
Hide resolved
110c38c
to
1cdec88
Compare
1cdec88
to
029d9e6
Compare
Used to prof and/or assure #11255 works. Seems it's more of testing AWS S3 Cli than Trino. Closing |
Hive data lake over proxy test
Description
Test Hive S3 datalake over proxy
Related issues, pull requests, and links
This PR was introduced to test #11255 And is adding integration test on top of this PR.
Documentation
(X) No documentation is needed.
(V) Sufficient documentation is included in this PR.
(X) Documentation PR is available with #prnumber.
(X) Documentation issue #issuenumber is filed, and can be handled later.
Release notes
(V) No release notes entries required.
(X) Release notes entries required with the following suggested text: