Add Presto container #2196

findepi · 2019-12-18T22:14:01Z

Default docker image

Should be a Docker Hub official image, or published by a reputable source (ideally the company or organisation that officially supports the technology)
Should have a verifiable open source Dockerfile and a way to view the history of changes
MUST show general good practices regarding container image tagging - e.g. we do not use latest tags, and we do not use tags that may be mutated in the future
MUST be legal for Testcontainers developers and Testcontainers users to pull and use. Mechanisms exist to allow EULA acceptance to be signalled, but images that can be used without a licence are greatly preferred.

Module dependencies

The module should use as few dependencies as possible,
Regarding libraries, either:
- they should be compileOnly if they are likely to already be on the classpath for users' tests (e.g. client libraries or drivers)
- they can be implementation (and thus transitive dependencies) if they are very unlikely to conflict with users' dependencies.
If client libraries are used to test or use the module, these MUST be legal for Testcontainers developers and Testcontainers users to download and use.

API (e.g. MyModuleContainer class)

Favour doing the right thing, and least surprising thing, by default
Ensure that method and parameter names are easy to understand. Many users will ignore documentation, so IDE-based substitutes (autocompletion and Javadocs) should be intuitive.
The module's public API should only handle standard JDK data types and MUST not expose data types that come from compileOnly dependencies. This is to reduce the risk of compatibility problems with future versions of third party libraries.

Documentation

Every module MUST have a dedicated documentation page containing:
A high level overview
A usage example
If appropriate, basic API documentation or further usage guidelines
Dependency information
Acknowledgements, if appropriate
Consider that many users will not read the documentation pages - even if the first person to add it to a project does, people reading/updating the code in the future may not. Try and avoid the need for critical knowledge that is only present in documentation.

bsideup · 2019-12-19T17:09:59Z

@findepi I just triggered a rerun

rnorth · 2019-12-26T13:32:35Z

Hi @findepi, thanks for taking time to start a contribution.

Please could you run through this section of the docs around new module contributions? https://www.testcontainers.org/contributing/#contributing-new-modules

It would be helpful if you could complete the checklist so that we can review efficiently! Thank you

findepi · 2019-12-28T14:58:00Z

@rnorth sure
i added the checklist to the PR message & filled it to the best of my knowledge.
The API section seems to be something you need to judge yourself, right?

I also added a docs page, following PostgreSQL and MariaDB examples.

Let me know what is the next step.

rushton · 2020-01-28T21:09:10Z

+1 looking forward to this

rnorth

Sorry this has taken a long time to get to. I'm happy with this, and I think we should proceed. I hope you don't mind but I pushed a couple of trivial docs-related tweaks to the PR branch already, and just have one final comment. I hope this is OK!

Thanks again for the contribution and all your hard work.

rnorth · 2020-02-07T21:14:31Z

modules/presto/src/test/java/org/testcontainers/containers/PrestoContainerTest.java

@@ -0,0 +1,138 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");


Testcontainers is MIT licenced, and it looks like this is the only file with an Apache header. Is that an error?

I'd prefer it if we could keep all source files similarly licenced, just to avoid confusion!

Thanks for pointing this out & apologies.

findepi · 2020-02-08T12:48:25Z

mkdocs.yml

@@ -53,6 +53,7 @@ nav:
                - modules/databases/neo4j.md
                - modules/databases/oraclexe.md
                - modules/databases/postgres.md
+                - modules/databases/presto.md


Thanks. I squash this with the first commit

findepi · 2020-02-08T12:48:38Z

docs/modules/databases/presto.md

@@ -1,5 +1,8 @@
 # Presto Module

+!!! note


findepi · 2020-02-08T12:51:32Z

modules/presto/src/test/java/org/testcontainers/containers/PrestoContainerTest.java

@@ -0,0 +1,138 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");


Thanks for pointing this out & apologies.

findepi · 2020-02-08T12:55:23Z

@rnorth thanks for the review! updated.

rnorth

@findepi perfect! Let's merge :D

* Add Presto container * Add incubating note to module docs Co-authored-by: Richard North <rich.north@gmail.com>

rnorth · 2020-03-05T16:26:16Z

Released in 1.13.0!

Thanks for the contribution @findepi 😃

* Add Presto container * Add incubating note to module docs Co-authored-by: Richard North <rich.north@gmail.com>

findepi requested review from bsideup, kiview and rnorth as code owners December 18, 2019 22:14

This comment has been minimized.

Sign in to view

bsideup added the type/new module label Dec 19, 2019

findepi force-pushed the presto branch 2 times, most recently from a85547b to d14243f Compare December 19, 2019 21:50

This comment has been minimized.

Sign in to view

findepi force-pushed the presto branch from d14243f to f4ae6c7 Compare December 22, 2019 07:59

findepi force-pushed the presto branch 2 times, most recently from 2e08dda to 5e46a43 Compare December 28, 2019 14:54

findepi force-pushed the presto branch from 5e46a43 to 9575f62 Compare January 5, 2020 15:54

findepi force-pushed the presto branch 6 times, most recently from 4bc6b36 to 08fa3e7 Compare January 27, 2020 21:40

findepi force-pushed the presto branch from 08fa3e7 to ec2fade Compare January 29, 2020 08:11

rnorth requested changes Feb 7, 2020

View reviewed changes

findepi and others added 2 commits February 8, 2020 13:51

Add Presto container

0b39d41

Add incubating note to module docs

508fb00

findepi commented Feb 8, 2020

View reviewed changes

findepi force-pushed the presto branch from 8e5999b to 508fb00 Compare February 8, 2020 12:52

rnorth approved these changes Feb 8, 2020

View reviewed changes

rnorth merged commit 61ce025 into testcontainers:master Feb 8, 2020

findepi deleted the presto branch February 8, 2020 20:56

artjomka pushed a commit to artjomka/testcontainers-java that referenced this pull request Feb 28, 2020

Add Presto container (testcontainers#2196)

62249f7

* Add Presto container * Add incubating note to module docs Co-authored-by: Richard North <rich.north@gmail.com>

rnorth added this to the 1.13.0 milestone Mar 5, 2020

quincy pushed a commit to quincy/testcontainers-java that referenced this pull request May 28, 2020

Add Presto container (testcontainers#2196)

502af4c

* Add Presto container * Add incubating note to module docs Co-authored-by: Richard North <rich.north@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Presto container #2196

Add Presto container #2196

findepi commented Dec 18, 2019 •

edited

This comment has been minimized.

bsideup commented Dec 19, 2019

This comment has been minimized.

rnorth commented Dec 26, 2019

findepi commented Dec 28, 2019

rushton commented Jan 28, 2020

rnorth left a comment

rnorth Feb 7, 2020

findepi Feb 8, 2020

findepi Feb 8, 2020

findepi Feb 8, 2020

findepi Feb 8, 2020

findepi commented Feb 8, 2020

rnorth left a comment

rnorth commented Mar 5, 2020

		@@ -0,0 +1,138 @@
		/*
		* Licensed under the Apache License, Version 2.0 (the "License");

Add Presto container #2196

Add Presto container #2196

Conversation

findepi commented Dec 18, 2019 • edited

Default docker image

Module dependencies

API (e.g. MyModuleContainer class)

Documentation

This comment has been minimized.

bsideup commented Dec 19, 2019

This comment has been minimized.

rnorth commented Dec 26, 2019

findepi commented Dec 28, 2019

rushton commented Jan 28, 2020

rnorth left a comment

Choose a reason for hiding this comment

rnorth Feb 7, 2020

Choose a reason for hiding this comment

findepi Feb 8, 2020

Choose a reason for hiding this comment

findepi Feb 8, 2020

Choose a reason for hiding this comment

findepi Feb 8, 2020

Choose a reason for hiding this comment

findepi Feb 8, 2020

Choose a reason for hiding this comment

findepi commented Feb 8, 2020

rnorth left a comment

Choose a reason for hiding this comment

rnorth commented Mar 5, 2020

findepi commented Dec 18, 2019 •

edited