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

Dev mode - make it possible to watch files matching a predicate #32980

Merged
merged 1 commit into from
May 15, 2023

Conversation

mkouba
Copy link
Contributor

@mkouba mkouba commented Apr 28, 2023

@mkouba mkouba requested review from stuartwdouglas and gsmet April 28, 2023 09:18
@mkouba mkouba force-pushed the dev-mode-watched-file-predicate branch from 1d7f5d2 to d7e4e0e Compare April 28, 2023 09:29
@Sanne
Copy link
Member

Sanne commented Apr 28, 2023

I haven't checked how this works, but at high level I wonder how this can possibly work. We don't watch every single file on the system, do we? So how can you possibly filter among the whole universe?

I suspect the term "predicate" is used in a different way than I'd expect, it might be worth clarifying at least javadoc, if not finding a better name.

@ia3andy
Copy link
Contributor

ia3andy commented Apr 28, 2023

I haven't checked how this works, but at high level I wonder how this can possibly work. We don't watch every single file on the system, do we? So how can you possibly filter among the whole universe?

I suspect the term "predicate" is used in a different way than I'd expect, it might be worth clarifying at least javadoc, if not finding a better name.

maybe naming it locationPredicate as the predicate is on the location?

@mkouba
Copy link
Contributor Author

mkouba commented Apr 28, 2023

I haven't checked how this works, but at high level I wonder how this can possibly work. We don't watch every single file on the system, do we? So how can you possibly filter among the whole universe?

@Sanne Of course we don't filter the whole universe. We do filter the set of detected changes. This set is not well documented but from the code it looks like we watch resource dirs of all reloadable modules (as defined in DependenciesFilter.getReloadableModules(ApplicationModel)).

I suspect the term "predicate" is used in a different way than I'd expect, it might be worth clarifying at least javadoc, if not finding a better name.

I will try to add something to the javadoc. Out of curiosity, what's the way you would expect?

@mkouba
Copy link
Contributor Author

mkouba commented Apr 28, 2023

I haven't checked how this works, but at high level I wonder how this can possibly work. We don't watch every single file on the system, do we? So how can you possibly filter among the whole universe?
I suspect the term "predicate" is used in a different way than I'd expect, it might be worth clarifying at least javadoc, if not finding a better name.

maybe naming it locationPredicate as the predicate is on the location?

Makes sense. I will also add some more info to the javadoc.

@mkouba mkouba force-pushed the dev-mode-watched-file-predicate branch from d7e4e0e to 2123e07 Compare April 28, 2023 11:35
@mkouba
Copy link
Contributor Author

mkouba commented Apr 28, 2023

We should also document what happens if multiple HotDeploymentWatchedFileBuildItems match the same file. Currently, the final value of isRestartNeeded equals to isRestartNeeded1 || isRestartNeeded2 . I think that we should use the same logic for predicates.

@mkouba mkouba force-pushed the dev-mode-watched-file-predicate branch from 2123e07 to 021c691 Compare April 28, 2023 13:36
@mkouba mkouba marked this pull request as ready for review April 28, 2023 13:36
@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@mkouba mkouba force-pushed the dev-mode-watched-file-predicate branch from 021c691 to 19c5691 Compare May 2, 2023 08:32
@quarkus-bot

This comment has been minimized.

@gsmet gsmet changed the title Dev mode - make it possibile to watch files matching a predicate Dev mode - make it possible to watch files matching a predicate May 2, 2023
- HotDeploymentWatchedFileBuildItem#builder().setPredicate(p).build()
@mkouba mkouba force-pushed the dev-mode-watched-file-predicate branch from 19c5691 to 5c659ef Compare May 3, 2023 13:32
@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@mkouba
Copy link
Contributor Author

mkouba commented May 4, 2023

Those JVM Tests - JDK 19 failures do not seem to be related...

@ia3andy
Copy link
Contributor

ia3andy commented May 5, 2023

I will give it a try today, thanks @mkouba

@mkouba
Copy link
Contributor Author

mkouba commented May 10, 2023

I will give it a try today, thanks @mkouba

Hi Andy, did you manage to try this out?

Copy link
Contributor

@ia3andy ia3andy left a comment

Choose a reason for hiding this comment

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

It's perfect @mkouba thanks again and sorry for the delay

@quarkus-bot

This comment has been minimized.

@mkouba
Copy link
Contributor Author

mkouba commented May 11, 2023

@gsmet I cannot make the JVM Tests - JDK 19 pass :-(. But the various failures do not look related...

@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@quarkus-bot
Copy link

quarkus-bot bot commented May 15, 2023

Failing Jobs - Building 5c659ef

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
JVM Tests - JDK 19 Download Maven Repo ⚠️ Check → Logs Raw logs

@mkouba mkouba added this to the 3.1 - main milestone May 15, 2023
@mkouba mkouba merged commit 78d3934 into quarkusio:main May 15, 2023
@ia3andy
Copy link
Contributor

ia3andy commented May 15, 2023

Awesome I was waiting for this one :) thanks agian @mkouba

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.

4 participants