fix: Use new model for Quarkus dev services#180
fix: Use new model for Quarkus dev services#180holly-cummins wants to merge 1 commit intomicrocks:new-dev-services-apifrom
Conversation
|
Thanks a lot @holly-cummins! Will dive into this in a few minutes.
Thanks a ton! |
Signed-off-by: Holly Cummins <[email protected]>
96a99df to
95a3fe5
Compare
lbroudoux
left a comment
There was a problem hiding this comment.
I have a few questions regarding build items vs. producers and the choice to have multiple MicrocksContainersEnsembleHostsBuildItem instances instead of just one. Can you help me better understand these points? Many thanks!
...oyment/src/main/java/io/github/microcks/quarkus/deployment/DevServicesMicrocksProcessor.java
Show resolved
Hide resolved
...oyment/src/main/java/io/github/microcks/quarkus/deployment/DevServicesMicrocksProcessor.java
Show resolved
Hide resolved
...oyment/src/main/java/io/github/microcks/quarkus/deployment/DevServicesMicrocksProcessor.java
Show resolved
Hide resolved
| .serviceName(MicrocksQuarkusProcessor.FEATURE + "-" + config.serviceName() + "minion") | ||
| .serviceConfig(config) // the lifecycle of the postman container should be the same as of the microcks container | ||
| .startable(microcksSupplier) | ||
| .dependsOnConfig(KAFKA_BOOTSTRAP_SERVERS, MinionContainerStartable::setKafkaBootstrapServersFromDevService) // the minion shouldn't be started until kafka is started |
There was a problem hiding this comment.
Is it possible to have multiple dependsOnConfig() like this? Will we be able to reuse the same mechanism if we started a RabbitMQ dev services in addition of the Kafka one?
There was a problem hiding this comment.
It is! Internally they're stored as a list, so you can add as many as you like.
You can declare them optional, which I've done here, so that the container will still be started even if the dependency never appears, or you can use the default call, which makes them mandatory. (Each dependency can be either optional or mandatory.)
Two caveats are:
- For now, they don't chain, because no one has asked for that yet. So if (say) you made a new 'supervisor' container that depended on RabbitMQ and then the minion container depended on the supervisor container, they wouldn't start in three stages
- The config that gets passed in is only from other dev services. So if, say, there was an external kafka service, the
start()method would have to read that config directly. I'm not sure that's intuitive so I might change this, what do you think?
There was a problem hiding this comment.
I'm not sure that's intuitive so I might change this, what do you think?
I understood this point only when reading the start() method on line 456. Actually, the dependsOnConfig() name might not be very accurate here... Maybe something like computeIfDevServiceConfigPresent() (analogous to Map.computeIfAbsent()) would be more helpful regarding the conditional behavior on dev service config...
If start() is called in any case, I tend to think that it's not really a dependency
Naming is hard for sure!
There was a problem hiding this comment.
If start() is called in any case, I tend to think that it's not really a dependency
The default is that start() wouldn't be called, but in the Microcks case, the minion needs to start even if there's no kafka config available, so I added the extra parameter for optional.
The confusion makes me think that neither the dependsOnConfig nor computeIfDevServiceConfigPresent name are quite right!
Resolves #161. Builds on https://github.com/microcks/microcks-quarkus/tree/new-dev-services-api.
This is a big diff, sorry! It will need Quarkus 3.31 (not yet released) and quarkusio/quarkus#51025 (not yet merged).
Here's what I've done:
Config.getConfig()at the time when the minion container is startingaPostmanCollectionIsPresentbecause static variables in processors are a bad idea; in this case the variable was only initialised in the poststartup hook, but it was used in a build step, which made the problem obvious.SimpleBuildItem, the docs say it has to return one or it’s an error.)With all this, the tests in
quarkus-microcks-demopass, and the dev ui looks about right. I did see some problems if I enable and then disable and then enable dev services but I don't know if that relates to my implementation here or is a more general issue.I did undo the change @lbroudoux has in my fork of
quarkus-microcks-demoto remove thequarkus.prefix from config, since this change seemed big enough on its own. :)