feat(DCH): Scaffold data-connect-hub module - #9451
Conversation
Generated-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Replace ESLint 9 flat config with ESLint 8 legacy config to match the project standard used by other packages (automl, autorag). The flat config format is incompatible with the current lint-staged setup. Changes: - Downgrade ESLint from 9.18.0 to 8.57.1 - Replace eslint.config.mjs with .eslintrc.cjs (legacy format) - Add .eslintignore for excluded directories - Remove ESLint 9-specific packages (@eslint/js, @eslint/markdown, typescript-eslint) - Add ESLint 8 compatible packages (@typescript-eslint/eslint-plugin, @typescript-eslint/parser) - Disable import/no-unresolved rule (TypeScript handles path alias resolution) - Fix linting errors in App.tsx: - Remove non-null assertions - Replace || with ?? (nullish coalescing) Generated-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![odh-dashboard-agent[bot]](https://avatars.githubusercontent.com/in/3714079?s=60&v=4){kind=small}
There was a problem hiding this comment.
Preflight Agent Report
Verdict: ❌ NOT READY
Commit: 2c8f80f
ℹ️ Draft PR — this report is informational. The PR is not expected to be merge-ready yet.
Checks
| Check | Status | Details |
|---|---|---|
| Conflicts | Mergeable but behind main — rebase before merge |
|
| CI — audit (data-connect-hub/frontend) | ❌ | 36 new npm advisory IDs introduced (new package dependencies) |
| CI — govulncheck (data-connect-hub/bff) | ❌ | 12 new Go vulnerability IDs introduced (new BFF dependencies) |
| CI — Konflux builds, check | Still running | |
| CI — Other checks | ⏭️ | Passed (quality gates, detect modules, etc.) |
| CodeRabbit | ➖ | Skipped — draft PR |
| Human review | No reviews yet (draft) | |
| Claude review | 1 minor — see inline comment | |
| Style review | 1 minor — see inline comment | |
| RBAC review | 1 minor, 1 nit — see details below | |
| Jira Eval | ➖ | RHAI-599 has no acceptance criteria |
| Jira | ✅ | RHAI-599 — active |
| Test Coverage | ✅ | 39 test files added (unit + Cypress) |
| PR Body | ❌ | Description, testing sections are empty (draft — fill before marking ready) |
CI Failures — action required
audit (packages/data-connect-hub/frontend) — 36 new npm advisories
The new packages/data-connect-hub/frontend package introduces 36 advisory IDs not present on the base branch. Because the package is brand-new, all its transitive npm dependency vulnerabilities register as "new". You'll need to either:
- Update the affected dependencies to fixed versions, or
- Add
npm auditoverrides in the package'spackage.jsonfor advisories that are known false-positives or not exploitable in this context.
Sample new advisory IDs: GHSA-22g5-r2x5-97cx, GHSA-25h7-pfq9-p65f, GHSA-337j-9hxr-rhxg (36 total — see CI logs for full list).
govulncheck (packages/data-connect-hub/bff) — 12 new Go vulnerabilities
Similarly, the new BFF's Go module graph introduces 12 new vulnerability IDs: GO-2026-4440, GO-2026-4441, GO-2026-4918, GO-2026-5024 … (12 total). Update Go dependencies to patched versions or add govulncheck exclusions for non-applicable findings.
🧹 Nitpick comments (1)
packages/data-connect-hub/bff/internal/integrations/kubernetes/k8mocks/base_testenv.go (1)
base_testenv.go: 🧹 Nit · RBAC review
Explicitly document that the mock's permission-error suppression is test-only. The existing comment notes that permission errors are intentionally ignored in the mock, but a build tag (//go:build !integration) or a more prominent comment block would prevent this pattern from being accidentally replicated in non-mock code.
Automated by ODH Dashboard Agent
| <PageSection | ||
| hasBodyWrapper={false} | ||
| isFilled | ||
| style={removeChildrenTopPadding ? { paddingTop: 0 } : undefined} |
There was a problem hiding this comment.
🟡 Minor · Style review
Replace inline style with a PF utility class.
style={removeChildrenTopPadding ? { paddingTop: 0 } : undefined} uses an inline style. Per project conventions, prefer PatternFly utility classes (e.g., className={removeChildrenTopPadding ? 'pf-v6-u-pb-0' : ''}) or a PF layout prop over inline style objects to keep theming consistent.
| var level slog.Level | ||
| err := level.UnmarshalText([]byte(s)) | ||
| if err != nil { | ||
| panic(fmt.Errorf("invalid log level: %s, valid levels are: error, warn, info, debug", s)) |
There was a problem hiding this comment.
🟡 Minor · Claude review
parseLevel() panics on invalid LOG_LEVEL instead of exiting gracefully.
Calling panic(...) on a misconfigured env var produces a crash dump rather than a clear startup error. Consider returning an error or calling os.Exit(1) with a descriptive message instead.
// current
panic(fmt.Errorf("invalid log level: %s, valid levels are: error, warn, info, debug", s))
// suggested
fmt.Fprintf(os.Stderr, "invalid log level %q — valid values: error, warn, info, debug\n", s)
os.Exit(1)1 participant
Issues
Description
How Has This Been Tested?
Test Impact
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main