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

hivemq > improve "temp-extensions" workaround #9501

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

SgtSilvio
Copy link
Contributor

@SgtSilvio SgtSilvio commented Nov 5, 2024

To support the withoutPrepackagedExtensions methods of the HiveMQContainer, we (HiveMQ) initially built in a workaround that copies extensions into /opt/hivemq/temp-extensions/ instead of /opt/hivemq/extensions/ and modifies the container entrypoint and command to move the extensions into their right place.

We have some upcoming changes to the HiveMQ container images that will remove the need for the withoutPrepackagedExtensions methods in most cases. This change applies the workaround only if any of the withoutPrepackagedExtensions methods are called.

This change also makes it easier to use the withCopyToContainer method with a Transferable argument to copy files into an HiveMQ extension's folder by just specifying /opt/hivemq/extensions/ as containerPath without needing to know about any implementation details. Currently, one is limited to methods like withFileInExtensionHomeFolder that apply the "temp-extensions" workaround internally, but these methods only support MountableFile arguments.

@SgtSilvio SgtSilvio requested a review from a team as a code owner November 5, 2024 14:39
@SgtSilvio SgtSilvio force-pushed the hivemq/improve-temp-extensions-workaround branch from a0197ba to bede900 Compare November 7, 2024 17:04
@SgtSilvio
Copy link
Contributor Author

Is there any way we can get merge rights if we just touch the hivemq module?

@SgtSilvio SgtSilvio force-pushed the hivemq/improve-temp-extensions-workaround branch from bede900 to c07314e Compare November 11, 2024 11:53
@SgtSilvio SgtSilvio force-pushed the hivemq/improve-temp-extensions-workaround branch 3 times, most recently from 101fdab to 0b1bbeb Compare November 21, 2024 09:51
@SgtSilvio
Copy link
Contributor Author

@eddumelendez Who should we talk to if we want to become code owners/maintainers of the hivemq module?

@SgtSilvio SgtSilvio force-pushed the hivemq/improve-temp-extensions-workaround branch from 0b1bbeb to 8bb6b00 Compare November 26, 2024 14:34
@eddumelendez
Copy link
Member

eddumelendez commented Nov 26, 2024

Thanks for the contribution, @SgtSilvio. But, we don't think this is a portable solution. Currently, HiveMQ module is provided in Java and NodeJS https://testcontainers.com/modules/hivemq/. Is there a way to hide this behind a env var? That way, no changes would be required in the module. Also, copyFileToContainer must not be overridden.

Who should we talk to if we want to become code owners/maintainers of the hivemq module?

The best person is @kiview

@SgtSilvio
Copy link
Contributor Author

HiveMQ module is provided in Java and NodeJS

This change tries to remove a workaround that is only present in the Java implementation. I don't see how the NodeJS implementation is affected because it does not have this workaround at all.

copyFileToContainer must not be overridden.

Can you explain me why?

@SgtSilvio
Copy link
Contributor Author

@kiview As the original contributors of the HiveMQ testcontainers module, we would like to know if it is possible to become maintainers/code owners/allowed reviewers of the hivemq module?
We do not ask for this because we simply want to merge any changes (we are very well concerned about backwards compatibility and don't want to introduce unnecessary changes), but we would like to remove some burden from your side, avoiding the need for you to understand the inner workings of HiveMQ and its container images.
What is your view on this and which options do you see?

@SgtSilvio SgtSilvio force-pushed the hivemq/improve-temp-extensions-workaround branch from 8bb6b00 to 1fef42b Compare December 9, 2024 08:13
@SgtSilvio SgtSilvio force-pushed the hivemq/improve-temp-extensions-workaround branch from 1fef42b to 83d25f7 Compare January 22, 2025 20:41
- Copy extensions to the temp dir only if required. It is required when removing prepackaged extensions.
- Custom copyToContainer calls that specify /opt/hivemq/extensions/... as container path automatically apply the temp dir workaround if required.
@SgtSilvio SgtSilvio force-pushed the hivemq/improve-temp-extensions-workaround branch from 83d25f7 to d1657cf Compare February 4, 2025 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants