-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add option to start Redis Dev Services on Null Host Value #41128
Comments
/cc @Ladicek (redis), @cescoffier (redis), @geoand (devservices,kubernetes), @iocanel (kubernetes), @machi1990 (redis), @stuartwdouglas (devservices) |
That should be the case already no? If the redis host is not configured, it should start the dev service. Or, do you mean you need the dev service when the programmatic host resolver returns null too. In that case, this is too late to start a dev service. The application is already running. |
@cescoffier I found the line and forked the Quarkus repo. This line says if the config property is present, don't start dev services. In my case, the property is present but pointed at an environment variable on the system, which may or may not be null. On my laptop, the environment variable is not set, so I would expect the dev services to start, however they do not. In my build server, because I cannot use containers, there is an instance of redis running that I can point to using the environment variable. Here's what's in my application properties
If the value is null, I'd like dev services to start, if the value is not null, I'd like dev services to not start. I understand this might represent a change in behavior, so I imagine there would need to be another flag to gate the changed behavior. Something along the lines of: if (redis-devservices-start-when-null-host && host-is-null) {
needStart = true;
} |
I see, it's not null. An environment variable cannot be defined and null. It can be empty. We can check for empty, and start the dev service in that case. Fancy a PR? You identified the line already. |
Well we should check for null and empty, just in case. |
@cescoffier I don't mind putting a PR in a little later this evening, it would be my first PR to Quarkus though, so go easy if I break some conventions. |
If the change is acceptable to be made to the LTS branch, would I just open a separate PR based on the 3.8 branch? |
No need, we backport automatically (if there is agreement to bring the change to LTS) |
I saw this a moment too late and ended up submitting two PRs. |
This is the case for almost every dev service by the way. Changing only for Redis would create inconsistency. |
That was I was thinking. But it seems that some dev services check for null or empty. We would need to check which dev service does this. |
Are you saying most extensions only start dev services on missing config property or that most start on missing or empty config property? I think missing-or-empty is a more flexible configuration, I don't mind bringing that change to other extensions as well if consistency is important. |
FWIW, I'm all for the change that was made in the PR and I think we should generalize it to all Dev Services checking a URL (or something similar). |
Most dev services use an utility method which checks for missing-or-empty, but without expression expansion. |
That would be super helpful for use cases like my own, where I want to test using a consistent profile on my local machine and my build servers, but only have access to a container runtime in one of those two environments. |
I can't seem to get the maven formatter to run on my system, but if you all would like, I can submit a separate PR later tonight to add that new method to the ConfigUtility. |
Removed self-referencing property expansion in oidc integration test Fixes quarkusio#41128
Removed self-referencing property expansion in oidc integration test Fixes quarkusio#41128
Removed self-referencing property expansion in oidc integration test Fixes quarkusio#41128
@ozangunalp what's the status on this one? |
Removed self-referencing property expansion in oidc integration test Fixes quarkusio#41128
Removed self-referencing property expansion in oidc integration test Fixes quarkusio#41128
Removed self-referencing property expansion in oidc integration test Fixes quarkusio#41128
Description
Some of my build processes occur inside containers in a staging Kubernetes environment. Dev services containers cannot be started in this environment.
On my normal development laptop however, dev services can be used to provide a Redis instance, a SQL Server, etc, nice!
One thing that I like about how certain extensions have implemented dev services, is the option to have them start if the provided hostname or host is null or empty.
This makes dev services a little more flexible on a per profile basis, which has allowed me to run my local tests using the same testing profile (%test) as my tests on my build containers in my staging environment.
Would it be possible to also add an option in the Redis extension to start dev services for Redis under a given set of conditions in a given profile?
In my case, I would want dev services to start in the dev profile always but in the test profile, if and only if the Redis hosts property is null/empty according to the smallrye config.
Implementation ideas
No response
The text was updated successfully, but these errors were encountered: