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

Add Apache Solr Module #2123

Merged
merged 34 commits into from
May 14, 2020
Merged

Add Apache Solr Module #2123

merged 34 commits into from
May 14, 2020

Conversation

raynigon
Copy link
Contributor

@raynigon raynigon commented Nov 30, 2019

Fixes #1562

The Goal is to add support for Apache Solr.

Checklist

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. (Apache License, see https://hub.docker.com/_/solr)

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.

@raynigon raynigon changed the title Add Apache Solr Module WIP: Add Apache Solr Module Nov 30, 2019
@raynigon raynigon changed the title WIP: Add Apache Solr Module Add Apache Solr Module Nov 30, 2019
* SolrContainer provides the TestContainer.
* SolrClientUtils can be used to create collections and upload configuration.
* SolrContainerConfiguration is a container to store all configuration options for the container.

Documentation was added in the docs/modules folder.
@raynigon
Copy link
Contributor Author

raynigon commented Nov 30, 2019

Hi @rnorth, @bsideup and @kiview,

i think the world needs to have solr Testcontainers :D
Here is my proposal for an implementation.
If you have any ideas how to improve the implementation, please let me know.
If this PR doesnt fit your criterias, please tell me what i can to to meet them.
Thanks for reviewing.
Have a nice day :D

@raynigon raynigon mentioned this pull request Jan 21, 2020

dependencies {
compile project(':testcontainers')
compile 'org.apache.httpcomponents:httpclient:4.5.10'
Copy link
Member

Choose a reason for hiding this comment

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

FYI to avoid an additional dependency, you can also use OkHttp that comes as a transitive dependency from the core module

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be fixed with 681a5e9

raynigon and others added 2 commits May 8, 2020 18:03
Replace the httpclient dependency with okhttp.
okhttp is already included in testcontainers.
Copy link
Member

@rnorth rnorth left a comment

Choose a reason for hiding this comment

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

Sorry for taking a long time to review. I've added some comments - mostly small things. Thanks for the contribution!

raynigon and others added 3 commits May 11, 2020 08:58
Co-authored-by: Richard North <rich.north@gmail.com>
Co-authored-by: Richard North <rich.north@gmail.com>
* Replaced localhost with containerIp
* Use codeinclude fpr documentation
* Ensure http response is closed
* Raise exception if core creation fails
* Remove author annotation
@raynigon raynigon requested a review from rnorth May 11, 2020 07:35
@raynigon raynigon requested a review from bsideup May 14, 2020 08:34
Copy link
Member

@rnorth rnorth 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 the updates @raynigon! This looks good to me, so I'm happy to approve. Thank you for the contribution and thanks for your patience!

@rnorth rnorth added this to the next milestone May 14, 2020
@rnorth rnorth merged commit f84bcd6 into testcontainers:master May 14, 2020
quincy pushed a commit to quincy/testcontainers-java that referenced this pull request May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apache Solr Support!
3 participants