bamaer commented on PR #8645:
URL: https://github.com/apache/hop/pull/8645#issuecomment-5855603869
Docs-only, so nothing here can break at runtime — none of this is a reason
to hold the PR up. The direction is good and the infographics are a clear step
up from the static PNGs. A few things are worth fixing first, mostly where the
new pages render worse than the ones they replace.
Verified against a local Antora build of `25ccb75` using `hop-website`'s
`antora-playbook-hop.yml`.
## Fix before merge
**1. Eight lists render as run-on paragraphs with literal `*` and `1.`**
No blank line between the lead-in sentence and the list, so Asciidoctor
folds the items back into the paragraph:
- `pipeline/pipelines.adoc` — lines 29, 48, 157
- `workflow/workflows.adoc` — lines 29, 45, 69, 109, 143
Built output for `pipelines.adoc:29`:
```html
<p>Pipelines are designed for high throughput and low latency: *
<strong>Ingestion</strong>: Read structured ... * <strong>Transformation &
Cleansing</strong>: ...</p>
```
This hits the main body copy of both new pages. GitHub's diff preview is
more forgiving than the Antora build, so it's easy to miss while writing —
reproduced with plain `asciidoctor` as well. Fix is a blank line before each of
the eight lists.
**2. The SVGs don't follow the site's theme toggle**
All three style dark mode with `@media (prefers-color-scheme: dark)` only;
none contains a `[data-theme]` selector. The Hop site switches theme via
`data-theme` on `<html>`, persisted in `localStorage['hop-theme']` —
`prefers-color-scheme` is only the initial default. So a visitor on a light OS
who clicks the dark toggle gets a dark page with three bright white
infographics, and the inverse on a dark OS toggled to light. Reproduced by
loading the built page with `data-theme="dark"` under a light OS preference.
`ui/css/tokens.css` has the pattern; each dark block needs a twin:
```css
@media (prefers-color-scheme: dark) { :root:not([data-theme="light"]) { /* …
*/ } }
:root[data-theme="dark"] { /* same rules */ }
```
Since these are `opts=inline`, `:root` resolves against the host page and
this works.
## Worth fixing
**3. Both load-balancing engine rows describe something the engine doesn't
do** — `workflows.adoc:98`, `pipelines.adoc:98`
`workflows.adoc:98` says *"Distributes child actions and workflows across a
cluster of Hop servers for high scalability and failover resilience."* Reading
through the engine:
- `LoadBalancingWorkflowEngine` extends `RemoteWorkflowEngine`.
`selectServer()` picks **one** server from the group, then
`super.submitToRemoteServer()` hands it the whole workflow — one assignment,
one container id. Actions are never spread across nodes. The plugin's own
description is *"Assigns the workflow to one Hop server from a configured
group."*
- There's no failover. The retry loop only retries on admission capacity
(`HopServerAdmission.isRetryableRegistrationFailure` matches
`HopServerAtCapacityException` / "at capacity"), and only while `containerId`
is still null — i.e. at submission. A server dying mid-run is not handled.
- One of the two algorithms is `pack` — *"Keep as few servers busy as
possible"* — which deliberately concentrates work rather than spreading it.
`validateRunConfigurationChain` is only a cycle guard for server-to-server
handoff (issue #4086), not distribution.
Suggested replacement: *"Assigns each workflow to one server from a
configured group, using an even-load or pack algorithm, and retries on another
server when the chosen one is at capacity."* Worth adding that child pipelines
and workflows **can** end up spread across the group, but that comes from the
child actions' own run configurations, not from this engine.
`LoadBalancingPipelineEngine` is the same shape, so `pipelines.adoc:98` needs
the same treatment.
**4. `workflow-backtracking-flow.svg` has two layout bugs.** The subtitle
overprints the title (*"Workflow Orchestration & Backtracking Mod**Actions**
run sequentially…"*), and the bottom caption is clipped at the viewBox edge,
ending mid-word on *"…follows the failure hop to"*.
**5. Dark-mode contrast in `hop-architecture-overview.svg`.** The non-accent
pill text — "Hop GUI", "hop-run (CLI)", "Python (PyHop)" and all four Storage
pills — stays dark navy on the dark background and is essentially unreadable.
`.pill-text` needs the dark override `.pill-accent` already has.
**6. "Your system never runs out of memory"** — `hop-gui-pipelines.adoc:34`,
and `pipelines.adoc:72`. Back-pressure only bounds the hop buffers; blocking
transforms still hold whole sets in memory. Hop's own `streamlookup.adoc` says
*"the entire lookup data set needs to fit in your available memory."* Suggest
scoping it to memory use *between* transforms.
**7. Prefix the SVG CSS class names.** `concepts.adoc` and
`getting-started/hop-concepts.adoc` inline all three SVGs on one page, so the
three `<style>` blocks become page-level CSS sharing `.bg`, `.card`,
`.text-title`, `.ribbon` and friends. It already shows: `.text-title` is 15px
in the pipeline SVG and 14.5px in the workflow SVG, so whichever inlines last
wins for both.
## Verified
No conversion errors or warnings from any changed page (the only warnings
are pre-existing `project_home` ones on unrelated transform pages). All 40 xref
targets and every image path in the changed files resolve; all three SVGs carry
Apache headers and inline correctly. Findings reproduced in the built HTML; 3
and 6 checked against the Java source.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
