quarkus icon indicating copy to clipboard operation
quarkus copied to clipboard

Add a Dev UI Screen for Agroal (DB)

Open phillip-kruger opened this issue 1 year ago • 23 comments

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

datasource

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

phillip-kruger avatar Oct 01 '24 06:10 phillip-kruger


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

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

:white_check_mark: 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.

quarkus-bot[bot] avatar Oct 01 '24 08:10 quarkus-bot[bot]

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!

FroMage avatar Oct 01 '24 10:10 FroMage


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

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

:white_check_mark: 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.

quarkus-bot[bot] avatar Oct 02 '24 01:10 quarkus-bot[bot]

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.

download_binary

@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 https://github.com/quarkusio/quarkus/issues/43721 for this
  • In another PR we will look (in general in Dev UI) to make sure all JsonRPC services are veto`ed

phillip-kruger avatar Oct 05 '24 05:10 phillip-kruger


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus Documentation CI

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

Failing Jobs

:warning: Artifacts of the workflow run were not available thus the report misses some details.

Status Name Step Failures Logs Raw logs Build scan
Documentation Build Build :warning: Check → Logs Raw logs :construction:

quarkus-bot[bot] avatar Oct 05 '24 06:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus Documentation CI

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

Failing Jobs

:warning: Artifacts of the workflow run were not available thus the report misses some details.

Status Name Step Failures Logs Raw logs Build scan
Documentation Build Build :warning: Check → Logs Raw logs :construction:

quarkus-bot[bot] avatar Oct 05 '24 07:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

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

:white_check_mark: 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.

quarkus-bot[bot] avatar Oct 05 '24 08:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus Documentation CI

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

Failing Jobs

:warning: Artifacts of the workflow run were not available thus the report misses some details.

Status Name Step Failures Logs Raw logs Build scan
Documentation Build Build :warning: Check → Logs Raw logs :construction:

quarkus-bot[bot] avatar Oct 05 '24 11:10 quarkus-bot[bot]

🎊 PR Preview fc237a6007d7e02a50000d0cd110c2df5ced39c4 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.

github-actions[bot] avatar Oct 07 '24 01:10 github-actions[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus Documentation CI

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

:white_check_mark: 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[bot] avatar Oct 07 '24 01:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

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

:white_check_mark: 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.

quarkus-bot[bot] avatar Oct 07 '24 02:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus Documentation CI

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

:white_check_mark: 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[bot] avatar Oct 14 '24 23:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

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

:white_check_mark: 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

:gear: Gradle Tests - JDK 17 Windows

:package: integration-tests/gradle

io.quarkus.gradle.devmode.KotlinProjectWithCompilerArgsDevModeTest.main - History

  • Condition with Lambda expression in io.quarkus.test.devmode.util.DevModeClient was not fulfilled within 2 minutes. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.test.devmode.util.DevModeClient was not fulfilled within 2 minutes.
	at app//org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at app//org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at app//org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at app//org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at app//org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at app//io.quarkus.test.devmode.util.DevModeClient.getHttpResponse(DevModeClient.java:164)
	at app//io.quarkus.gradle.devmode.QuarkusDevGradleTestBase.getHttpResponse(QuarkusDevGradleTestBase.java:164)

quarkus-bot[bot] avatar Oct 15 '24 00:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus Documentation CI

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

:white_check_mark: 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[bot] avatar Oct 15 '24 02:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

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

:white_check_mark: 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.

quarkus-bot[bot] avatar Oct 15 '24 03:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus Documentation CI

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

:white_check_mark: 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[bot] avatar Oct 15 '24 04:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

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

:white_check_mark: 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.

quarkus-bot[bot] avatar Oct 15 '24 05:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus Documentation CI

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

:white_check_mark: 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[bot] avatar Oct 17 '24 04:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus Documentation CI

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

:white_check_mark: 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[bot] avatar Oct 17 '24 07:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

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

:white_check_mark: 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.

quarkus-bot[bot] avatar Oct 17 '24 08:10 quarkus-bot[bot]

@maxandersen @cescoffier can we make a call on this ?

phillip-kruger avatar Oct 17 '24 23:10 phillip-kruger


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus Documentation CI

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

:white_check_mark: 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[bot] avatar Oct 17 '24 23:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

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

:white_check_mark: 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

:gear: JVM Tests - JDK 17 Windows

:package: extensions/opentelemetry/deployment

io.quarkus.opentelemetry.deployment.metrics.HttpServerMetricsTest.collectsHttpRouteFromEndAttributes - History

  • Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.untilAsserted(ConditionFactory.java:790)
	at io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter.assertCountPointsAtLeast(InMemoryMetricExporter.java:131)
	at io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter_ClientProxy.assertCountPointsAtLeast(Unknown Source)
  • Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Assertion condition defined as a Lambda expression in io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter expected: <true> but was: <false> within 5 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.untilAsserted(ConditionFactory.java:790)
	at io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter.assertCountPointsAtLeast(InMemoryMetricExporter.java:131)
	at io.quarkus.opentelemetry.deployment.common.exporter.InMemoryMetricExporter_ClientProxy.assertCountPointsAtLeast(Unknown Source)

quarkus-bot[bot] avatar Oct 18 '24 00:10 quarkus-bot[bot]

Where are we on this? Have all the concerns been addressed?

cescoffier avatar Oct 23 '24 14:10 cescoffier

we won't be able to make it something more than half baked.

It could stay half-baked because we don't have the resources to improve it, but the community might pick it up and turn it into something magical. Chances are that if it's useful, people will improve it.

FroMage avatar Oct 23 '24 17:10 FroMage

@yrodiere - The client side check is also done on the server side. The client side is just there to prevent a user doing something that we know can not be done (usability rather than security).

@yrodiere - I'll also make a note that you will not maintain this, and make sure you are not being bothered with any bugs / request on this. I'll take care of this as much as possible.

@gsmet - I would not call this half baked. Yes it's not a full blown SQL client, but that's is not what it is. It's a basic table viewer. We can definitely add more features going forward, but I think this is a good start.

phillip-kruger avatar Oct 24 '24 00:10 phillip-kruger

@yrodiere - The client side check is also done on the server side. The client side is just there to prevent a user doing something that we know can not be done (usability rather than security).

Ah, sorry. Indeed I see it now. Thanks for the clarification.

@gsmet - I would not call this half baked. Yes it's not a full blown SQL client, but that's is not what it is. It's a basic table viewer. We can definitely add more features going forward, but I think this is a good start.

I think the "half baked" comment was not about what you did -- I don't think we could do much better. It's more about the fact that this covers the basics, and that going further, covering the many specific cases of each JDBC driver and how they handle types, is simply not achievable -- or at least, not worth the effort, especially since it's duplicated effort (other tools exist).

@yrodiere - I'll also make a note that you will not maintain this, and make sure you are not being bothered with any bugs / request on this. I'll take care of this as much as possible.

Ok then. Thanks, and good luck :)

Let's see what Max thinks of it.

yrodiere avatar Oct 24 '24 07:10 yrodiere

make sure you are not being bothered with any bugs / request on this. I'll take care of this as much as possible

Famous Last Words if I ever heard any 🤣

BTW, just to make that clear, it's a big +1 from me, this PR.

FroMage avatar Oct 24 '24 14:10 FroMage


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus Documentation CI

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

:white_check_mark: 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[bot] avatar Oct 30 '24 09:10 quarkus-bot[bot]


:waning_crescent_moon: This workflow status is outdated as a new workflow run has been triggered.


Status for workflow Quarkus CI

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

:white_check_mark: 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.

quarkus-bot[bot] avatar Oct 30 '24 10:10 quarkus-bot[bot]