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.

Sign up for GitHub

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

Jump to bottom

Refactor TestcontainersConfiguration to allow config by env var #3411

Merged
merged 1 commit into from Oct 29, 2020
Merged

Refactor TestcontainersConfiguration to allow config by env var #3411

merged 1 commit into from Oct 29, 2020

Conversation

rnorth
Copy link
Member

@rnorth rnorth commented Oct 29, 2020

Split out from #3102

@rnorth rnorth requested review from bsideup and kiview October 29, 2020 11:45
@rnorth rnorth changed the title Refactor Testcontainers configuration to allow config by env var Refactor TestcontainersConfiguration to allow config by env var Oct 29, 2020
rnorth added a commit that referenced this pull request Oct 29, 2020
@rnorth
Add Image substitution mechanism
950af34 
Builds upon #3021 and #3411:

* adds a pluggable image substitution mechanism using ServiceLoader, enabling users to perform custom substitution/auditing of images being used by their tests

* provides a default implementation that behaves similarly to legacy `TestcontainersConfiguration` approach (`testcontainers.properties`)

Notes:

* behaviour is similar but not quite identical to `TestcontainersConfiguration`: use of a configured custom image for, e.g. Kafka/Pulsar that does not have a tag specified causes the substitution to take effect for all usages. It seems very unlikely that people would be using a mix of the config file image overrides in some places _and_ specific images specified in code in others.

* Duplication of default image names in modules vs `TestcontainersConfiguration` class is intentional: specifying image overrides in `testcontainers.properties` should be removed in the future.

* ~Add log deprecation warnings when `testcontainers.properties` image overrides are used.~ Defer to a future release?
rnorth added a commit that referenced this pull request Oct 29, 2020
@rnorth
Add Image substitution mechanism
58179b3 
Builds upon #3021 and #3411:

* adds a pluggable image substitution mechanism using ServiceLoader, enabling users to perform custom substitution/auditing of images being used by their tests

* provides a default implementation that behaves similarly to legacy `TestcontainersConfiguration` approach (`testcontainers.properties`)

Notes:

* behaviour is similar but not quite identical to `TestcontainersConfiguration`: use of a configured custom image for, e.g. Kafka/Pulsar that does not have a tag specified causes the substitution to take effect for all usages. It seems very unlikely that people would be using a mix of the config file image overrides in some places _and_ specific images specified in code in others.

* Duplication of default image names in modules vs `TestcontainersConfiguration` class is intentional: specifying image overrides in `testcontainers.properties` should be removed in the future.

* ~Add log deprecation warnings when `testcontainers.properties` image overrides are used.~ Defer to a future release?
@rnorth rnorth mentioned this pull request Oct 29, 2020
Merged
@rnorth rnorth added this to the 1.15.0 milestone Oct 29, 2020
bsideup
bsideup approved these changes Oct 29, 2020
View reviewed changes
Copy link
Member

@bsideup bsideup left a comment

Choose a reason for hiding this comment

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

👍

@rnorth rnorth merged commit ca74b04 into master Oct 29, 2020
@rnorth rnorth deleted the config-via-env-vars branch October 29, 2020 13:12
rnorth added a commit that referenced this pull request Nov 5, 2020
@rnorth
Image substitution (#3102)
44e8e9a 
* Refactor Testcontainers configuration to allow config by env var

* Add Image substitution mechanism

Builds upon #3021 and #3411:

* adds a pluggable image substitution mechanism using ServiceLoader, enabling users to perform custom substitution/auditing of images being used by their tests

* provides a default implementation that behaves similarly to legacy `TestcontainersConfiguration` approach (`testcontainers.properties`)

Notes:

* behaviour is similar but not quite identical to `TestcontainersConfiguration`: use of a configured custom image for, e.g. Kafka/Pulsar that does not have a tag specified causes the substitution to take effect for all usages. It seems very unlikely that people would be using a mix of the config file image overrides in some places _and_ specific images specified in code in others.

* Duplication of default image names in modules vs `TestcontainersConfiguration` class is intentional: specifying image overrides in `testcontainers.properties` should be removed in the future.

* ~Add log deprecation warnings when `testcontainers.properties` image overrides are used.~ Defer to a future release?

* Remove extraneous change

* Un-ignore docs example test by implementing a 'reversing' image name substitutor

* Use configuration, not service loader, to select an ImageNameSubstitutor

* Add check for order of config setting precedence

* Extract classpath scanner and support finding of multiple resources

* Introduce deterministic merging of classpath properties files

* Update docs

* Update docs

* Remove service loader reference

* Chain substitution through default and configured implementations

* Small tweaks following review

* Fix test compile error

* Add UnstableAPI annotation

* Move TestSpecificImageNameSubstitutor back to original package and remove duplicate use of default substitutor
@ailjushkin
Copy link

ailjushkin commented Oct 12, 2022 •
edited

@rnorth Hello, could you please explain why the property for reusing a container could be set only through the env or user property but not through the classpath property?

@UnstableAPI
    public boolean environmentSupportsReuse() {
        // specifically not supported as an environment variable or classpath property
        return Boolean.parseBoolean(getEnvVarOrUserProperty("testcontainers.reuse.enable", "false"));
    }

Why not setting this in the classpath file?

@kiview
Copy link
Member

kiview commented Oct 12, 2022

@ailjushkin Reusable mode is an experimental opt-in feature, and now allowing it on the classpath is done for CI safety reasons. You can find additional explanation here: #5364 (comment)

@ailjushkin
Copy link

@ailjushkin Reusable mode is an experimental opt-in feature, and now allowing it on the classpath is done for CI safety reasons. You can find additional explanation here: #5364 (comment)

Thanks for the quick answer, I got it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bsideup bsideup bsideup approved these changes

@kiview kiview Awaiting requested review from kiview

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.15.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@rnorth @ailjushkin @kiview @bsideup