-
-
Notifications
You must be signed in to change notification settings - Fork 526
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat(azure): add Azurite, EventHubs and ServiceBus in the new Azure module, deprecating the old Azurite module #3008
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
* main: docs: refine texts on how to set the module image (testcontainers#3012) feat(modules): add dind module (testcontainers#3004) docs: correct container variable (testcontainers#3010) chore: update Weaviate version to v1.29.0 and Weaviate Go client to v5.0.2 (testcontainers#3006)
This reverts commit fbf78a0.
@@ -65,7 +65,7 @@ jobs: | |||
|
|||
- name: ensure compilation | |||
working-directory: ./${{ inputs.project-directory }} | |||
run: go build | |||
run: go build ./... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed because the new azure module has subpackages for each container type.
@stevenh I'm tempted to follow the same approach for the GCP module: to have sibpackages for each container type, so they all have their Run
function, and not RunBigQuery
, etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a good idea to me.
modules/azure/azurite/azurite.go
Outdated
// AzuriteContainer represents the Azurite container type used in the module | ||
type AzuriteContainer struct { | ||
testcontainers.Container | ||
Settings options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping this public to avoid the breaking change. OTOH, if we want to do the BC now, I'm fine to move this settings to private.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should take the hit now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's never used, so we can fairly remove it.
* main: chore(deps): bump github.com/opencontainers/image-spec from 1.1.0 to 1.1.1, dario.cat/mergo from 1.0.0 to 1.0.1 (testcontainers#3030) chore(deps): bump github/codeql-action from 3.28.0 to 3.28.11 (testcontainers#3014) chore(deps): bump ossf/scorecard-action from 2.4.0 to 2.4.1 (testcontainers#3013) chore: readd dependabot, including a way to refresh the project files for all the modules (testcontainers#2997)
@@ -65,7 +65,7 @@ jobs: | |||
|
|||
- name: ensure compilation | |||
working-directory: ./${{ inputs.project-directory }} | |||
run: go build | |||
run: go build ./... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a good idea to me.
|
||
const ( | ||
// BlobPort is the default port used by Azurite | ||
BlobPort = "10000/tcp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: do these need to be exported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kept the same state as in the original, deprecated modules/azurite
container.
modules/azure/azurite/azurite.go
Outdated
// AzuriteContainer represents the Azurite container type used in the module | ||
type AzuriteContainer struct { | ||
testcontainers.Container | ||
Settings options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should take the hit now.
// MustServiceURL returns the URL of the given service, panics if an error occurs | ||
func (c *AzuriteContainer) MustServiceURL(ctx context.Context, srv Service) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: do we need the must, I would remove if not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's used in the examples, or you mean we should not add this kind of Must methods.
return fmt.Sprintf(connectionStringFormat, hostPort, defaultSharedAccessKeyName, defaultSharedAccessKey), nil | ||
} | ||
|
||
// MustConnectionString returns the connection string for the eventhubs container, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: do we need?
What does this PR do?
This PR is doing two things:
azure
, in which the new Azure containers will live, each on its own subpackage. In examplemodules/azure/eventhubs
.modules/azurite
module, in favor ofmodules/azure/azurite
. The feature has been migrated to the new module, and the deprecated code is linking to the new module, including the imports.Regarding the new module containers, we are adding:
Azurite
As mentioned, deprecates the former Azurite module, using the same features. It also adds more robust wait strategies for the different service types (Bloc, Queue and Table).
EventHubs
The EventHubs emulator needs an Azurite container, so the module starts it under the hood. For that:
As a result, container communication happens using the aliases and the well-known ports. For the end-users, it's transparent, and they just have to talk to the EventHubs container using the provided API. The testable examples include how to connect to the container.
ServiceBus
The ServiceBus emulator needs a MSSQL Server container, so the module starts it under the hood. For that:
As a result, container communication happens using the aliases and the well-known ports. For the end-users, it's transparent, and they just have to talk to the ServiceBus container using the provided API. The testable examples include how to connect to the container.
Why is it important?
More cloud modules! this time to verify interactions with Azure Cloud.
Follow-ups
I'm not porting the CosmosDB emulator module, as it's broken in the upstream. I tried to run the tc-java implementation and it fails: