Add a Dev UI Screen for Agroal (DB)

[phillip-kruger]

This PR adds a Dev UI screen for Agroal (so any time you have a DB).

You can view a more detailed demo here: https://youtu.be/-ms_2ayumkk

[gsmet]

The idea is nice and I can definitely see the value but I wonder if it won't open a can of worms. Data is hard and displaying the data coming directly from a table is not that easy - see my comments inline for a few cases where things might go wrong.

Now, what I don't know is if it's just a matter of tweaking things a bit or if it will be a continuous stream of corner cases to fix.

/cc @yrodiere

[maxandersen]

first all - love the feature and something I did some early experiments on (but without the ui skills :)

I have concerns though that is similar to @gsmet and something we at least should consider before pushing in, in no particular order:

1. which schema are you listing? the default the connection is for or all of them ?
   with JPA on top users can very specifically limit what data is ever exposed but here its all raw...

2. data security - we already have concerns about exposing config data in devui; but now we will potentially expose a query interface to all data in the raw jdbc connection on the agroal connection? ...should we turn this off by default or at least have option to turn on? (maybe separate devmode only extension?)

3. feature creep - this will open up for massive set of feature requests and can keep us busy forever. We should either agree to deliberately keep this very simple or explore if more suitable reuse of some existing webbased db ui ?

[yrodiere]

Thanks, looking good -- though I added a few comments below.

This PR adds a Dev UI screen for Agroal (so any time you have a DB).

Note Agroal is not necessarily present if you have a reactive datasource. You might need a similar Dev UI for reactive SQL clients...

Now, what I don't know is if it's just a matter of tweaking things a bit or if it will be a continuous stream of corner cases to fix.

I suspect there will be lots of corner cases, though whether we get to hear about it will depend on the success of this feature.

I know that I would rather work on #39584 than fix an endless stream of bugs related to JDBC quirks -- Hibernate at least hides some of them.

But I agree this is a (very!) nice start, so perhaps we could merge this as a "preview" feature, and depending on feedback we'll decide whether we keep it as is, or try to migrate it to something more "Hibernate-focused" as part of #39584? E.g. if data types are really too hard to handle, Hibernate could help (a lot).

[FroMage]

Looks like a great feature! Now, as @yrodiere said, I want to be able to run HQL queries, not just SQL ones ;)
Oh, and also click from the table definition to the entity in the IDE?
But congrats, this is looking great!

[phillip-kruger]

Ok, there was a few comments from multiple people. I have done the following updates:

@yrodiere

Note Agroal is not necessarily present if you have a reactive datasource. You might need a similar Dev UI for reactive SQL clients..

Yes, if this is requested we can look at something similar for reactive drivers.

I know that I would rather work on #39584 than fix an endless stream of bugs related to JDBC quirks -- Hibernate at least hides some of them.

I think we should still have a Hibernate Dev UI Presence (and the log stream :) ) - however for this one the aim is to give a basic DB View. I think we should have both.

What I did:

• Removed the health check excluded names.
• Added a JsonRPC Tests case.
• Added a loading bar when we load the db details.
• Change to page over data - we might still have a problem if the driver loads everything into memory and the DB is massive.

@FroMage

W.r.t HQL queries, this is something we will need to add to the Hibernate extension.
W.r.t byte[] (bytea) like images, I have now added support for those.

@gsmet

• We now page over that result - we might still have a problem if the driver loads everything into memory and the DB is massive.
• bytea are now allowing download (see above)

@max

which schema are you listing? the default the connection is for or all of them ?

The schema(s) the user of the connection (as defined in the properties) have access too. We might need to fine tune this for dev service DB. ( I get this from Agroal)

data security - we already have concerns about exposing config data in devui; but now we will potentially expose a query interface to all data in the raw jdbc connection on the agroal connection? ...should we turn this off by default or at least have option to turn on? (maybe separate devmode only extension?)

There is now a option to turn this off totally (default false) or turn on/off the sql query part (default off). So by default you will not be able to run queries. You need to turn this on in config. Apart from that (after chatting to @yrodiere) I also only allow this feature on local databases (so this feature will not be enabled for datasouces that is not local)

So not only do we have the LocalHostFilter for dev UI that only allow requests when the host is localhost, but now for this I also made it:

• Local dev only (no remote dev)
• Local DB only - so this will only work if the DB itself is also localhost.

feature creep - this will open up for massive set of feature requests and can keep us busy forever. We should either agree to deliberately keep this very simple or explore if more suitable reuse of some existing webbased db ui ?

As with anything else in Quarkus, we will make a call on every feature request. The plan is too keep it simple. So real complex things users will still use an external tool (like their IDE)

@cescoffier

• Removed from Menu, as discussed, this is now on the card. I create Allow any link n Dev UI Menu #43721 for this
• In another PR we will look (in general in Dev UI) to make sure all JsonRPC services are veto`ed

[github-actions]

🎊 PR Preview fc237a6 has been successfully built and deployed to https://quarkus-pr-main-43618-preview.surge.sh/version/main/guides/

• Images of blog posts older than 3 months are not available.
• Newsletters older than 3 months are not available.

[phillip-kruger]

We have a green CI !!

[aloubyansky]

As discussed with Phillip, I added a commit on top that configures the resolver for dev mode in QuarkusDevModeTest. That's the reason why quarkus-agroal-dev wasn't enabled in the logging-panache IT tests and had to be added a test scoped dependency.

[aloubyansky]

Those are flaky test failures. They passed locally for me.

[quarkus-bot]

Status for workflow Quarkus Documentation CI

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

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

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

[quarkus-bot]

Status for workflow Quarkus CI

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

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

📦 extensions/opentelemetry/deployment

✖ io.quarkus.opentelemetry.deployment.traces.OpenTelemetrySpanSecurityEventsTest.testSecurityEventTypes - History

• event executor terminated - java.util.concurrent.RejectedExecutionException

java.util.concurrent.RejectedExecutionException: event executor terminated
	at io.netty.util.concurrent.SingleThreadEventExecutor.reject(SingleThreadEventExecutor.java:934)
	at io.netty.util.concurrent.SingleThreadEventExecutor.offerTask(SingleThreadEventExecutor.java:353)
	at io.netty.util.concurrent.SingleThreadEventExecutor.addTask(SingleThreadEventExecutor.java:346)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:836)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute0(SingleThreadEventExecutor.java:827)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:817)
	at io.vertx.core.impl.EventLoopExecutor.execute(EventLoopExecutor.java:35)

Concerns addressed.

[michalvavrik]

This annotation is used for maps only according to the docs https://smallrye.io/smallrye-config/Main/config/mappings/#withdefaults. Does it have any function here?

[michalvavrik]

Why is this test dependency added here? I can't see any other file or change in this module. Maybe it deserves a comment above the dependency for others to understand why is it present.

[aloubyansky]

This dependency was removed in a follow up PR a84b285

[michalvavrik]

Alright, thank you.

[michalvavrik]

While it's cool you can do that, the <artifactId>quarkus-agroal-dev</artifactId> is hardcoded anyway and if someone is looking for usage (or simply trying to figure how it works) of the quarkus-agroal-dev, they won't find it. If the artifactId ever changes, then also will have to change the quarkus-agroal-dev so I think this is unnecessary and it would be easier to read if the value was hardcoded.

(and you already have this dependency name hardcoded in the deployment module)

[michalvavrik]

FYI inner-records are always static

[michalvavrik]

Sorry, this can be stupid question but I didn't try to run this new cool feature yet, what happens if I do select timestamp_create from products, this this contains("create ") matches?