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

Make LocalStack image configurable #1873

Merged
merged 1 commit into from
Sep 23, 2019
Merged

Conversation

aosagie
Copy link
Contributor

@aosagie aosagie commented Sep 14, 2019

Hi,
In order to use Test Containers and LocalStack at my company, we need to resolve images from our corporate registry. Currently, LocalStackContainer's image is hard coded. This PR makes it configurable. I followed the lead of a prior commit that made KafkaContainer's image configurable: 4e15af82

I noticed a prior PR attempted to do this: #1604
But

  1. Its development seems to have been stalled for the past couple months
  2. It's slightly more complex than necessary

Thanks for your time!

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 contributing @aosagie!
I think this looks fine really. It obviously lacks a test, but it's clear enough to see its behaviour and consistent with the other image name properties.

@aosagie
Copy link
Contributor Author

aosagie commented Sep 19, 2019

Thanks for the prompt review @rnorth
Is there anything additional I need to do to get this merged or is everything good to go?

@rnorth
Copy link
Member

rnorth commented Sep 23, 2019

@aosagie sorry no, nothing needed - wanted to give other reviewers a chance to look before merging, but will merge myself now.

@rnorth rnorth merged commit 41a009d into testcontainers:master Sep 23, 2019
@rnorth
Copy link
Member

rnorth commented Sep 23, 2019

Merged. Thank you for the contribution @aosagie!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants