-
Notifications
You must be signed in to change notification settings - Fork 3k
Fix checking for docker enviroment when quarkus.devservices.enabled=false
#49309
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
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The failures look suspicious. |
I'll look at failures today and report will report back. |
I'll also have a look. The failing tests were enabled by #48787, so I suspect that, rather than #47610, may be responsible for the excessive checking. Since the failing tests didn't ever work before #48787, disabling them again wouldn't be totally unreasonable, at least until we find the right tradeoff between "too much checking if dev services are enabled" and "not enough checking if dev services are enabled." But hopefully we can find a way to get the best of both. |
I looked at it and I think as I added the devservice enabled check on To reproduce it (for more visual explanation)
Possible solution what worked for me to fix original issue with windows was adding the devservices enable check on config buildstep Like this @BuildStep(onlyIf = { IsDevelopment.class, DevServicesConfig.Enabled.class })
public List<DevServiceDescriptionBuildItem> config( Could this be viable solution @holly-cummins or you see some drawback/potentional problems? |
Status for workflow
|
My initial thought was that it was totally sensible to guard something that reads and writes dev service config with a guard to only do it when dev services are enabled. My only worry with this is we have a few cases where extensions ignore that property and do dev-services things anyway. I'll see if I can find a specific example, since my memory about the pattern is pretty vague. Even if I'm right that it does happen, the right fix might be to change those extensions, because ignoring the config seems like a recipe for chaos. Edit: I've just checked, and can only find one case where we start dev services even when they're disabled, which is in Lambda. |
Thanks for the fix! |
I hope this makes it into 3.25.1! |
Thanks, I'll push it to the next 3.25. |
Fixes #49118
Not sure if the whole
DevServicesProcessor
build steps should be skipped. Second option to fix the issue is addingDevServicesConfig.Enabled.class
to https://github.com/quarkusio/quarkus/blob/main/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java#L162But in general it seems this error is caused by https://github.com/quarkusio/quarkus/blob/main/extensions/devservices/deployment/src/main/java/io/quarkus/devservices/deployment/DevServicesProcessor.java#L172