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.

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

Allow to asynchronously notify extensions of no-restart changes #40682

Merged
merged 1 commit into from
May 17, 2024

Conversation

ia3andy
Copy link
Contributor

@ia3andy ia3andy commented May 16, 2024

This allows a clean way to notify extensions of no-restart changes like there: quarkiverse/quarkus-web-bundler#217:

RuntimeUpdatesProcessor.INSTANCE
                .notifyExtensions(noRestartChanges)

This also allows to reset the compileProblem asynchronously.

For context, when the change is already handled/compiled by the underlying process, triggering a doScan() is not necessary and could even be an issue. Still, extensions needs to be notified of the change.

@ia3andy ia3andy requested review from FroMage and stuartwdouglas May 16, 2024 12:59
@ia3andy ia3andy changed the title Allow processors to notify extensions of no-restart changes Allow to asynchronously notify extensions of no-restart changes May 16, 2024
@ia3andy
Copy link
Contributor Author

ia3andy commented May 16, 2024

Tested with web-bundler

This comment has been minimized.

@ia3andy ia3andy force-pushed the improve-runtime-update branch from 5a08fc6 to 9f763bb Compare May 17, 2024 07:36
Copy link
Member

@FroMage FroMage left a comment

Choose a reason for hiding this comment

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

Looks good.

@ia3andy ia3andy force-pushed the improve-runtime-update branch from 9f763bb to bf266dc Compare May 17, 2024 07:58
Copy link

quarkus-bot bot commented May 17, 2024

Status for workflow Quarkus CI

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

✅ 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.

You can consult the Develocity build scans.


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)

@ia3andy ia3andy merged commit d77ee69 into quarkusio:main May 17, 2024
51 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.12 - main milestone May 17, 2024
@ia3andy ia3andy deleted the improve-runtime-update branch May 17, 2024 13:15
Comment on lines +600 to +601
scanLock.lock();
codeGenLock.lock();
Copy link
Member

Choose a reason for hiding this comment

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

Why the new locks?

Copy link
Member

Choose a reason for hiding this comment

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

For the context, I want to make sure this won't cause problems as you asked for a backport.

@gsmet gsmet modified the milestones: 3.12 - main, 3.11.0 May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants