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.

Sign up for GitHub

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

Jump to bottom

Allow quarkus:run to launch DevServices #40273

Merged
merged 2 commits into from
Apr 25, 2024
Merged

Allow quarkus:run to launch DevServices #40273

merged 2 commits into from
Apr 25, 2024

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Apr 25, 2024 •
edited
Loading

@quarkus-bot quarkus-bot bot added area/core area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/maven labels Apr 25, 2024
@geoand geoand mentioned this pull request Apr 25, 2024
Closed
aloubyansky
aloubyansky reviewed Apr 25, 2024
View reviewed changes
core/deployment/src/main/java/io/quarkus/deployment/cmd/RunCommandProcessor.java Outdated
@@ -52,7 +52,7 @@ public void defaultJavaCommand(PackageConfig packageConfig,
List<String> args = new ArrayList<>();
args.add(determineJavaPath());

for (Map.Entry<?, ?> e : System.getProperties().entrySet()) {
for (Map.Entry<?, ?> e : System.getProperties().entrySet()) { //TODO: this is almost certainly wrong as it pulls in all the system properties Maven has set
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could select only those that are found on the command line. There is also some relevant code in DevMojo. E.g. we have systemProperties as a Maven plugin goal parameter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/quarkusio/quarkus/blob/3.10.0/devtools/maven/src/main/java/io/quarkus/maven/DevMojo.java#L1422
and perhaps some of https://github.com/quarkusio/quarkus/blob/3.10.0/devtools/maven/src/main/java/io/quarkus/maven/DevMojo.java#L1247-L1284

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... As this is code that runs in the Quarkus build system, I am wondering if we should use BuildSystemTargetBuildItem.buildSystemProps

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that should be it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR updated

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point, it will rely on a Maven-specific env var to get the command line

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could enhance it later

Copy link
Contributor Author

@geoand geoand Apr 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So do you propose I add it or not? Gotcha

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I need to figure out hot to import the gradle plugins into IntelliJ :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the Gradle change as well

@quarkus-bot quarkus-bot bot added the area/gradle Gradle label Apr 25, 2024
@geoand geoand marked this pull request as ready for review April 25, 2024 13:45
@geoand geoand requested a review from aloubyansky April 25, 2024 16:15
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Apr 25, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 9e6ddc6.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 extensions/smallrye-reactive-messaging-kafka/deployment

✖ io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase.sseStream - History

  • Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase Expecting size of: [] to be greater than or equal to 2 but was 0 within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: 
Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase 
Expecting size of:
  []
to be greater than or equal to 2 but was 0 within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)

@aloubyansky aloubyansky merged commit 9b4f221 into quarkusio:main Apr 25, 2024
62 of 64 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone Apr 25, 2024
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Apr 25, 2024
@geoand geoand deleted the #40270 branch April 26, 2024 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aloubyansky aloubyansky aloubyansky approved these changes

Assignees
No one assigned
Labels
area/core area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle area/maven kind/enhancement New feature or request release/noteworthy-feature triage/flaky-test
Projects
None yet
Milestone
3.11.0.CR1
Development

Successfully merging this pull request may close these issues.

Introduce DevServices in quarkus:run Maven goal
2 participants
@geoand @aloubyansky