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

Fix dev mode when the same file is marked multiple times for hot deploy #13727

Merged
merged 1 commit into from Dec 7, 2020

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented Dec 6, 2020

Better reviewed commit per commit.

Fixes #13722

Also changes the behavior for named persistence units (which is slightly related as we will then only produce the import.sql build item once).

/cc @Sanne I would need your blessing for the change of behavior in the second commit. My rationale is that there's a good chance you will not want to push the same import.sql for your additional named datasources.

This can happen and we should gracefully handle this.

Fixes quarkusio#13722
@gsmet
Copy link
Member Author

gsmet commented Dec 6, 2020

@geoand can you check the first commit?

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

First commit looks good!

Copy link
Member

@Sanne Sanne left a comment

Choose a reason for hiding this comment

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

I'm ok with it if you feel strongly about it, but I'm not 100% persuaded that this is the right thing to do.

I agree that it's unlikely to be useful for the same import script to be imported by multiple PUs, but this same "oddness" has been areound for a long time in vanilla Hibernate as well.

So the difference in Quarkus is that we auto-configure the default one; the big question we don't really know is what if people prefer using a named PU (even though they have onlyone), and what's the experience when migrating from a default PU to a named PU ?

I'll approve as I have no strong opinion, but I'm a bit reserved that this will create confusion and maybe just requiring people to override each property explicitly is simpler to handle.

For sure will need some better explanation in migration docs.

@gsmet
Copy link
Member Author

gsmet commented Dec 7, 2020

I /think/ it's the best thing to do but let's wait for someone to complain.

I dropped the second commit.

@gsmet gsmet merged commit ecd5e6d into quarkusio:master Dec 7, 2020
@ghost ghost added this to the 1.11 - master milestone Dec 7, 2020
@gsmet gsmet modified the milestones: 1.11 - master, 1.10.3.Final Dec 7, 2020
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.

Error on quarkus:dev after upgrade to 1.10.2.Final only with multiple persistence units
3 participants