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 autoUpdateExternalUrl to CloudStorageEmulatorContainer #825

Merged
merged 2 commits into from
Aug 31, 2024

Conversation

alecmev
Copy link
Contributor

@alecmev alecmev commented Aug 27, 2024

Previously discussed here: #805 (comment)

Defaulting to true because it's very likely that the container will be used locally, and that it's desired for the emulator to return valid URLs. If an externalUrl is provided then the update is skipped.

Can't get tests to work with Colima (I added forListeningPorts but it hangs in InternalPortCheck.isBound, here), so need a CI run to check.

Copy link

netlify bot commented Aug 27, 2024

Deploy Preview for testcontainers-node ready!

Name Link
🔨 Latest commit 90decae
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-node/deploys/66d07ed3124d810008b39d25
😎 Deploy Preview https://deploy-preview-825--testcontainers-node.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Comment on lines -103 to -108
public getExternalUrl(): string {
if (this._externalURL) {
return this._externalURL;
} else {
return this.getEmulatorEndpoint();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this fallback because undefined is a valid state (i.e., "not set"), and _externalURL will usually be up-to-date, thanks to updateExternalUrl.

@cristianrgreco cristianrgreco added enhancement New feature or request minor Backward compatible functionality labels Aug 28, 2024
@alecmev
Copy link
Contributor Author

alecmev commented Aug 28, 2024

Both errors appear to be unrelated to this PR, do I need to do anything?

@cristianrgreco cristianrgreco merged commit b93fdfb into testcontainers:main Aug 31, 2024
94 checks passed
@alecmev alecmev deleted the auto-update-external-url branch August 31, 2024 11:32
@alecmev
Copy link
Contributor Author

alecmev commented Aug 31, 2024

Thank you 👍

@cristianrgreco
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request minor Backward compatible functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants