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

Remove unconfigured datasource #36687

Closed
wants to merge 2 commits into from
Closed

Remove unconfigured datasource #36687

wants to merge 2 commits into from

Conversation

jtama
Copy link
Contributor

@jtama jtama commented Oct 25, 2023

Fixes #36666

@quarkus-bot
Copy link

quarkus-bot bot commented Oct 25, 2023

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should not end up with dot

This message is automatically generated by a bot.

@jtama jtama changed the title Remove unconfigured datasource. Remove unconfigured datasource Oct 25, 2023
@gsmet gsmet marked this pull request as draft October 25, 2023 12:49
@jtama
Copy link
Contributor Author

jtama commented Oct 25, 2023

So I manually dispatched the build but it skipped everything. Don't know why. And my laptop is dying so I can't build all quarkus ;)

@yrodiere
Copy link
Member

So I manually dispatched the build but it skipped everything. Don't know why. And my laptop is dying so I can't build all quarkus ;)

The workflow ignores the main branch in forks, don't ask me why. You need to work in a branch that is not named main...

@quarkus-bot quarkus-bot bot added area/hibernate-orm Hibernate ORM area/persistence OBSOLETE, DO NOT USE labels Oct 25, 2023
@jtama
Copy link
Contributor Author

jtama commented Oct 25, 2023

So I had to amend hibernate extension that had to explicitely take care of UnconfiguredDataSource.

@jtama
Copy link
Contributor Author

jtama commented Oct 26, 2023

@jtama I think your build is still failing? https://github.com/jtama/remove-unconfigured-data-source/actions/runs/6649954232/job/18069255620

See https://github.com/quarkusio/quarkus/blob/main/CONTRIBUTING.md#ide-config-and-code-style

Yes indeed I did spoke to quickly. Fixing all this and now correcting the real errors which are for the moment, extension that needed to explicitly handle unconfigured datasources as error cases...

@yrodiere
Copy link
Member

Yes indeed I did spoke to quickly. Fixing all this and now correcting the real errors which are for the moment, extension that needed to explicitly handle unconfigured datasources as error cases...

Maybe we should list those and discuss this more widely before you spend too much time on it, as we're not sure yet this is the right approach... please ping me when you get a full build listing all errors.

@jtama
Copy link
Contributor Author

jtama commented Oct 26, 2023

@yrodiere
Copy link
Member

Looking at the tests, I realized that this removal of UnconfiguredDatasource is hitting a few problems because "deactivating" datasources is sometimes necessary, but the only way we provide to do this is implicit, by not providing connection info.

I'll try to implement a way to explicitly "deactivate" a datasource through a configuration property as discussed on quarkus-dev, and will send a PR. We'll see where this leads us.

Thanks for the initial investigation @jtama!

@yrodiere
Copy link
Member

See also #37265

@jtama jtama closed this by deleting the head repository Jan 3, 2024
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/agroal area/hibernate-orm Hibernate ORM area/persistence OBSOLETE, DO NOT USE triage/invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Health check on io.quarkus.agroal.runtime.UnconfiguredDataSource always reports healthy
2 participants