-
-
Notifications
You must be signed in to change notification settings - Fork 568
feat: add Solace pubsub+ module #3230
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
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
@unicod3 thanks for submitting this PR. I did an initial review and found some issues that need to be addressed, mostly related to the consistent patterns we apply when building the modules.
TL;DR;
- no constructor for the Container type
- options must be exposed at the package level, not added to the container type
- Run function using functional opts
In any case, we can work with you in making it consistent, and merge it into the project. Thank you so much for your contribution
@mdelapenya I've addressed them all, let me know if I missed anything, thank you for the review. |
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.
Looks pretty good, thanks for your hard work! Just a few comments, and we are done 😊
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.
LGTM! Even though I think we need to use testify in the options_test file, I'm approving this PR before hand.
Thank you for all your hard work during the review 🙇
You're welcome, enjoyed the collaboration. In my last commit, I also addressed linter issues, there is a CodeQL check failing however I am not really sure if that's because of my branch. |
@mdelapenya just fixed the failing test issue fyi |
@unicod3 I added a commit on top of yours, just to refine the docs. PLMK what you think. We can merge this PR right after that. |
LGTM. |
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.
Pull Request Overview
This PR adds a new Testcontainers module for Solace Pubsub+, providing a ready-to-use container interface for testing applications that use Solace messaging services. The module supports configuration of multiple service types (AMQP, MQTT, REST, Management, SMF), queue/topic setup, and authentication options.
Key changes:
- Implements core Solace container functionality with configurable options for credentials, VPN, services, and message queues
- Provides service-specific URL generation for different Solace protocols (AMQP, MQTT, REST, SMF)
- Includes comprehensive test coverage and examples demonstrating message publishing and consumption
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
modules/solace/solace.go | Core container implementation with Run function, service configuration, and CLI script execution |
modules/solace/service.go | Service definitions for AMQP, MQTT, REST, Management, and SMF protocols |
modules/solace/options.go | Configuration options for credentials, VPN, queues, services, and shared memory |
modules/solace/solace_test.go | Basic integration test validating container startup and service accessibility |
modules/solace/options_test.go | Unit tests for all configuration options and their combinations |
modules/solace/examples_test.go | Example usage with Solace SDK integration for message publishing/consuming |
modules/solace/mounts/solace.script.tpl | CLI script template for queue and topic configuration |
docs/modules/solace.md | Module documentation with usage examples and API reference |
Comments suppressed due to low confidence (1)
docs/modules/solace.md:54
- The function name should be 'WithVPN' (all caps) to match the actual function name in the code, not 'WithVpn'.
#### WithVpn
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.
LGTM, thanks!
@unicod3 thanks for your contribution! I'm adding it to the modules catalog right now: testcontainers/community-module-registry#147 |
…util/v4-4.25.6 * main: fix(dockermcpgateway): use duckduckgo instead of brave (#3247) feat: add Solace pubsub+ module (#3230) feat(options): add WithProvider (#3241) chore(deps): bump github/codeql-action from 3.29.2 to 3.29.3 (#3237) chore(deps): bump golang.org/x/oauth2 in /modules/weaviate (#3240) chore(deps): bump mkdocs-include-markdown-plugin from 7.1.5 to 7.1.6 (#3239) chore(deps): bump requests from 2.32.0 to 2.32.4 (#3204) feat(mcpgateway): add MCP gateway module (#3232) chore(deps): bump golang.org/x/oauth2 in /modules/pulsar (#3236) chore(deps): bump golang.org/x/oauth2 in /modules/gcloud (#3235) chore(deps): bump golang.org/x/oauth2 in /modules/k3s (#3234) chore: prepare for next minor development cycle (0.39.0) chore: use new version (v0.38.0) in modules and examples Update go.mod in azure module (#3231) fix: strip headers from logs using log stream specification (#3226) chore: clarify image auth warning message for public images (#3228) chore(deps): bump github.com/go-viper/mapstructure/v2 (#3219) chore(deps): bump github/codeql-action from 3.28.16 to 3.29.2 (#3222) chore(deps): bump mkdocs-include-markdown-plugin from 7.1.5 to 7.1.6 (#3225)
What does this PR do?
Adds a new module for Solace pubsub+
Why is it important?
Exposes a ready-to-use module for Solace Pubsub+
Related issues