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

Restart app on pom.xml change #4896

Merged
merged 1 commit into from
Oct 29, 2019
Merged

Conversation

stuartwdouglas
Copy link
Member

Unlike other hot replacement this simply watches the pom.xml files,
and reloads the complete application on change. This means there is
a short period where the app in unavailible.

Fixes #4871

Map<Path, Long> pomFiles = readPomFileTimestamps(runner);
for (;;) {
//we never suspend after the first run
suspend = "n";
Copy link
Member

Choose a reason for hiding this comment

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

Hello @stuartwdouglas, is this meant to be passed along to the new DevModeRunner, so that it doesn't (start and) suspend in debug mode? If so, I don't see changing this value playing a role in it, since the args being passed to the new DevModeRunner already have a bunch of JVM -Xdebug options setup in it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I will just move all this logic into prepare()

Unlike other hot replacement this simply watches the pom.xml files,
and reloads the complete application on change. This means there is
a short period where the app in unavailible.

Fixes quarkusio#4871
@emmanuelbernard
Copy link
Member

@stuartwdouglas I think it works well I tested it in a few combinations and never had unexpected behaviors. Except in one case:

  • create a new RESTEasy project (from generator)
  • add mvn quarkus:add-extensions -Dextensions="jdbc-postgresql,hibernate-orm-panache"
  • add Todo class with @entity on it
  • refresh see error
  • remove jdbc-postgresql,hibernate-orm-panache fromt he pom
  • refresh and don't see error even though the Todo app has missing packages and annotation references that would not compile

We can survive without but that's a bit of an unexpected bahavior.

Also needs testing on Windows I think @maxandersen

Copy link
Member

@emmanuelbernard emmanuelbernard left a comment

Choose a reason for hiding this comment

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

Stuart I approved it with two pending conditions:

  • review my "non compilation" error comment and see if that's an easy fix
  • I asked @maxandersen to test this PR on Windows, it's probably smart to make sure it works on W before pushing into the release. He will od it today asap after one platform issue is behind us

@emmanuelbernard
Copy link
Member

@stuartwdouglas ^

@stuartwdouglas
Copy link
Member Author

There is nothing I can do about that short of issuing a 'clean compile' invocation after pom changes. Even then I am not really sure how I would go about reporting the error.

I don't think this is a big deal as mostly we care about adding extensions rather than removing them.

@stuartwdouglas stuartwdouglas added this to the 0.27.0 milestone Oct 29, 2019
@maxandersen
Copy link
Member

first thing - if you clean compile don't you risk loosing any post-compile generated artifacts in the target dir ? or are they assumed not relevant for devmode ?

@stuartwdouglas
Copy link
Member Author

dev mode running happens straight after compile, so if there are any they will be out of date anyway. I don't think it is really a workable solution, and I don't think it is a major problem.

@emmanuelbernard
Copy link
Member

Yes I'm cool with that little quirk and see how it develops in practical usages

@stuartwdouglas stuartwdouglas merged commit 4e5675e into quarkusio:master Oct 29, 2019
@maxandersen
Copy link
Member

for the record i tested the basic flow and worked on windows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Detect pom changes when in quarkus:dev mode
4 participants