NO-JIRA: Bump to 4.22.0-prerelease.3 SDK#903
Conversation
|
@logonoff: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (3)
WalkthroughBumped three dynamic-plugin-sdk packages to a new prerelease; switched CSRF token import to the SDK internal package and added a Cypress dummy exporter returning undefined; removed a webpack alias override that blocked resolving Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/components/dashboards/perses/perses/datasource-cache-api.ts`:
- Line 14: The build replaces `@openshift-console/dynamic-plugin-sdk-internal`
with cypress/dummy-sdk-internal.tsx which currently lacks getCSRFToken, causing
runtime errors when datasources-cache-api.ts calls getCSRFToken; add a mock
export named getCSRFToken to cypress/dummy-sdk-internal.tsx that returns a safe
value (e.g., a string token or null) so calls from the function that sets the
CSRF header succeed in standalone/Cypress mode; ensure the export signature
matches usage (no args, returns string | null) and is exported as a named
export.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 822b4efa-c0d3-4f09-b9cc-bd59f58b0373
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
web/package.jsonweb/src/components/dashboards/perses/perses/datasource-cache-api.tsweb/webpack.config.ts
💤 Files with no reviewable changes (1)
- web/webpack.config.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/dashboards/perses/perses/datasource-cache-api.ts (1)
100-109:⚠️ Potential issue | 🟠 MajorUse a collision-free cache key.
Concatenating
kind,name, andprojectwith-can collide for valid values that themselves contain hyphens, which would make the cache return the wrong datasource or suppress a needed lookup.♻️ Proposed fix
private generateKey(selector: DatasourceSelector, project?: string): string { - let key = selector.kind; - if (selector.name !== undefined) { - key += `-${selector.name}`; - } - if (project !== undefined) { - key += `-${project}`; - } - return key; + return JSON.stringify([selector.kind, selector.name ?? null, project ?? null]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/dashboards/perses/perses/datasource-cache-api.ts` around lines 100 - 109, The generateKey function builds cache keys by concatenating selector.kind, selector.name and project with hyphens which can collide when those values contain hyphens; change generateKey (in datasource-cache-api.ts) to produce a collision-free key by serializing the components deterministically (e.g. JSON.stringify({kind: selector.kind, name: selector.name, project})) or by joining them with a non-allowed delimiter (like '\0') or by prefixing each component with a label (e.g. "k:", "n:", "p:") so that lookups using the DatasourceSelector always map to a unique key.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@web/src/components/dashboards/perses/perses/datasource-cache-api.ts`:
- Around line 100-109: The generateKey function builds cache keys by
concatenating selector.kind, selector.name and project with hyphens which can
collide when those values contain hyphens; change generateKey (in
datasource-cache-api.ts) to produce a collision-free key by serializing the
components deterministically (e.g. JSON.stringify({kind: selector.kind, name:
selector.name, project})) or by joining them with a non-allowed delimiter (like
'\0') or by prefixing each component with a label (e.g. "k:", "n:", "p:") so
that lookups using the DatasourceSelector always map to a unique key.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9021c0a6-d214-445c-98c0-0eb4e71ffa40
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
web/cypress/dummy-sdk-internal.tsxweb/package.jsonweb/src/components/dashboards/perses/perses/datasource-cache-api.tsweb/webpack.config.ts
💤 Files with no reviewable changes (1)
- web/webpack.config.ts
✅ Files skipped from review due to trivial changes (2)
- web/package.json
- web/cypress/dummy-sdk-internal.tsx
|
/lgtm |
|
@logonoff: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jgbernalp, logonoff, vojtechszocs The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Follow up to #876 which provides a more "official" path to consume
getCSRFTokenSummary by CodeRabbit