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

Issue 32595 - Kafka Streams Dev UI migration to v2 #36650

Closed
wants to merge 5 commits into from

Conversation

dcotfr
Copy link
Contributor

@dcotfr dcotfr commented Oct 23, 2023

Proposal of rewriting of Kafka Streams Dev UI for v2.

Replaces Mermaid usage by a wasm version of Graphviz (seems lighter and with less dependencies).

Still generates a mermaid graph definition (for backward compatibility) and adds a new digraph definition for Graphviz.

Warning : contains a direct link to https://cdn.jsdelivr.net/npm/@hpcc-js/wasm/dist/graphviz.js in qwc-kafka-streams-topology.js, because I don't know how to include this dependency with in a more cleaner / secured way.

fixes #32595

@dcotfr dcotfr changed the title Issue 32595 - Kafka Streams Dev UI migration to v2 proposal Issue 32595 - Kafka Streams Dev UI migration to v2 Oct 23, 2023
@dcotfr dcotfr marked this pull request as ready for review October 23, 2023 22:20
@gsmet gsmet requested a review from phillip-kruger October 24, 2023 07:55
@gsmet
Copy link
Member

gsmet commented Oct 24, 2023

Thanks a lot, I pinged @phillip-kruger so that he can have a look.

Copy link
Member

@phillip-kruger phillip-kruger left a comment

Choose a reason for hiding this comment

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

LGTM, I think we can merge once the change is made

import { unsafeHTML } from 'lit/directives/unsafe-html.js';
import { JsonRpc } from 'jsonrpc';

import { Graphviz } from "https://cdn.jsdelivr.net/npm/@hpcc-js/wasm/dist/graphviz.js";
Copy link
Member

Choose a reason for hiding this comment

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

Here you can use import { Graphviz } from "@hpcc-js/wasm/graphviz"; and add this to deployment/pom.xml:

<dependency>
        <groupId>org.mvnpm.at.hpcc-js</groupId>
        <artifactId>wasm</artifactId>
        <version>2.14.1</version>
        <scope>runtime</scope>
      </dependency>

You can also add above to build parent pom dependency management and reference in the extension pom without the version

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes done and everything seems Ok when used in sample project.

image

@quarkus-bot

This comment has been minimized.

@quarkus-bot quarkus-bot bot added the area/dependencies Pull requests that update a dependency file label Oct 25, 2023
@dcotfr dcotfr requested a review from phillip-kruger October 25, 2023 18:03
@phillip-kruger
Copy link
Member

@dcotfr Thanks for this ! Please rebase and squash your PR, and then on green CI we can merge

@quarkus-bot

This comment has been minimized.

@phillip-kruger
Copy link
Member

@dcotfr seems like the wasm js dependency pulls in a lot of other dependencies with ranges. To make our build faster we lock these in the build-parent. I'll send you (a bit later) what needs to be added to the build parent.

@dcotfr dcotfr closed this Oct 25, 2023
@dcotfr dcotfr deleted the issue-32595 branch October 25, 2023 23:44
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file area/kafka-streams triage/invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dev UI: Migrate Kafka streams to the new Dev UI
3 participants