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

Misc[docker]: Adding dependency of bmqbrkr to bmqtool in docker-compose.yaml single-node example #475

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

pts95
Copy link
Contributor

@pts95 pts95 commented Oct 25, 2024

*Issue number of the reported bug or feature request: #476 *

Describe your changes
Great project!

Testing the examples listed on the official Getting Started page on a M1 Macbook Pro (darwin/arm64) and I notice the build and example instructions don't work.

Think this is due to two reasons:

  1. The project fails to build due to many compiler warnings. Temporarily commenting out -DBDE_BUILD_SAFE=on works.
  2. The instructions on starting the bmqtool in the doc link above doesn't work - start fails to connect, because the bmqbrkr is not started. This is probably because bmqtool doesn't depend on bmqbrkr service container - which it should.

This PR fixes the 2nd issue by adding a dependency on bmqbrkr in bmqtool via depends_on. I am still going through how broker health checks work - it should be service_healthy instead, but I couldn't find an easy way to do this in the bmqtool CLI help. So using service_started instead - to only start bmqtool container once

Partially addresses #476

Testing performed
Follow all the steps listed here with workaround suggested in 1 and the depends_on changes in this PR - everything from building and testing the queue should work as intended.

Additional context
N/A. I might make more PRs - this project looks interesting.

@pts95 pts95 requested a review from a team as a code owner October 25, 2024 00:16
@678098 678098 self-assigned this Oct 25, 2024
@678098 678098 self-requested a review October 25, 2024 13:38
@678098
Copy link
Collaborator

678098 commented Oct 25, 2024

Hi @pts95, thanks for the PR. It's a good fix
Currently the CI fails because the PR title needs to be changed, so it starts with one of the allowed prefixes. I propose to start the title with Misc[docker]: ... We use these preset prefixes to automatically sort release notes.
Another problem is DCO check. It's a common problem, and we put an instruction what it is and how to fix this fast here: https://github.com/bloomberg/blazingmq/blob/main/CONTRIBUTING.md#dco-check

@pts95 pts95 changed the title Adding dependency of bmqbrkr to bmqtool in docker-compose.yaml single-node example Misc[docker]: Adding dependency of bmqbrkr to bmqtool in docker-compose.yaml single-node example Oct 25, 2024
…docker compose file

Signed-off-by: Aditya Kumar Praharaj <[email protected]>
Copy link
Collaborator

@pniedzielski pniedzielski 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 the change! I'm going to let CI run once more to be safe, but looks good to merge in.

@pniedzielski
Copy link
Collaborator

Test failure seems unrelated; will look into in a different changeset.

@pniedzielski pniedzielski merged commit dfba278 into bloomberg:main Nov 5, 2024
29 of 30 checks passed
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.

3 participants