search

quarkiverse / quarkus-amazon-services

Quarkus Amazon Services extensions
Apache License 2.0
41 stars 49 forks source link

Specifying additional localstack services #370

Closed kevinross closed 1 year ago

kevinross commented 1 year ago

I noted that properties for localstack get passed to it via quarkus.aws.devservices.localstack.container-properties.[*]=*, so I tried using this to add a SERVICES value in the hope that devservices would see that and append others to it rather than simply clobbering it with the values from the BuildItems.

Spoiler: it clobbers it.

I've looked around and couldn't find anything that would help. The "golden path" of writing an extension to do this with a DevServicesLocalStackProviderBuildItem is way overkill^(1) for the use case of enabling additional localstack services IMO^(2).

Would this clobbering of SERVICES be considered a bug? If not, consider this my vote for it to be one and to start the conversation.

(1) not to mention issues including the extension's gradle project into a non-extension gradle project so it can be used without having to publish it, but I digress (2) that's not to say that extensions should not be written, just that for the use case of "enabling extra localstack services" it's not necessary

scrocquesel commented 1 year ago

Services should normally be handled automatically depending the services used in the project. Are you adding extra services because you need them to interact with an another project that used it ? Would you need it for both test mode and dev mode ?

kevinross commented 1 year ago

My use case involves services not already implemented as an extension; ie if I wanted to have localstack enable the service, I'd have to write the quarkus extension for it first, even if I had no plans to use the extension and was only using it to configure localstack.

If the extension existed, I'd depend on it, create a no-op class to expose the API client, and mark it unremovable so it'd be kept during build.

If quarkus respected an existing config property (presumably set via env var, but the entire config source hierarchy is in scope by design) for $SERVICES and appended to it instead of overwriting, that would solve this problem.

TBD would be making sure it was exposed outside of the container (if it needed extra ports, etc, how quarkus would deal with that, if at all), but I'd be happy with just having the service enabled and leaving me to my own devices to get it exposed to the outside world.

EDIT: for dev or test mode? Both, probably. If I'm trying to use one localstack service during testing, I can't see myself not using it for dev too.

scrocquesel commented 1 year ago

@kevinross Can you give a try to the PR ? (mvn install the branch and use the 999-SNAPSHOT version).

You will need to add the additional services like

quarkus.aws.devservices.localstack.additional-services.redshift.enabled=true
quarkus.aws.devservices.localstack.additional-services.kinesis.enabled=true

Those are managed like any others so dev services will provide config property for those services as well (like "quarkus.redshift.endpoint-override)

kevinross commented 1 year ago

Cloned, built, tested and the additional service is passed in!

Apparently, testcontainers defaults to "EAGER_SERVICE_LOADING=0", which means "SERVICES" is ignored entirely and all services are "available" but not started/enabled; this means if quarkus keeps the default as-is, this work isn't needed :( , but if the default is ever changed (ie to make enabling services explicit vs implicit), this work would be needed.

(I'm in the "explicit over implicit" camp, but won't lose any sleep if the default is kept)

scrocquesel commented 1 year ago

Cloned, built, tested and the additional service is passed in!

Apparently, testcontainers defaults to "EAGER_SERVICE_LOADING=0", which means "SERVICES" is ignored entirely and all services are "available" but not started/enabled; this means if quarkus keeps the default as-is, this work isn't needed :( , but if the default is ever changed (ie to make enabling services explicit vs implicit), this work would be needed.

(I'm in the "explicit over implicit" camp, but won't lose any sleep if the default is kept)

Right, SERVICES is also now deprecated (https://docs.localstack.cloud/references/configuration/#deprecated). LocalStack seems to have done some perf improvement and made lazy starting of services a defacto. I guess we can keep calling withServices for a while for those using older version of the docker image.

But then, I wonder why you can't access services not added to the container SERVICES ?

kevinross commented 1 year ago

I probably could have, I hadn't actually looked at the container's logs until now (combination of not needing to and an assumption that only enabled services will work, because of the SERVICES var), just the logs from java.

That said, probably a good idea to still have this option for the reason you mentioned. Also, if localstack gates any more features behind a pro subscription (as is their right, I'm not opposed to people making money!), it lets us use an older version that might need this capability.

On Sat, Jan 14, 2023, 11:07 a.m. Sébastien CROCQUESEL < @.***> wrote:

Cloned, built, tested and the additional service is passed in!

Apparently, testcontainers defaults to "EAGER_SERVICE_LOADING=0", which means "SERVICES" is ignored entirely and all services are "available" but not started/enabled; this means if quarkus keeps the default as-is, this work isn't needed :( , but if the default is ever changed (ie to make enabling services explicit vs implicit), this work would be needed.

(I'm in the "explicit over implicit" camp, but won't lose any sleep if the default is kept)

Right, SERVICES is also now deprecated ( https://docs.localstack.cloud/references/configuration/#deprecated). LocalStack seems to have done some perf improvement and made lazy starting of services a defacto. I guess we can keep calling withServices for a while for those using older version of the docker image.

But then, I wonder why you can't access services not added to the container SERVICES ?

— Reply to this email directly, view it on GitHub https://github.com/quarkiverse/quarkus-amazon-services/issues/370#issuecomment-1382836302, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3MOUNCKOE2BS4ALDVTPVLWSLFNBANCNFSM6AAAAAASBI6344 . You are receiving this because you were mentioned.Message ID: @.***>

gsmet commented 1 year ago

Fixed by https://github.com/quarkiverse/quarkus-amazon-services/pull/472