feat(experimentation): Snowflake warehouse frontend#7566
Conversation
…imentation-warehouse-setup-frontend
…tion menu, edit support
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Docker builds report
|
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
|
Visual Regression19 screenshots compared. See report for details. |
|
@gemini-code-assist review please |
There was a problem hiding this comment.
Code Review
This pull request adds support for creating and updating external warehouse connections, specifically Snowflake. It includes new API mutations, updated TypeScript types, and a new modal for connection management. The warehouse tab UI has been updated to handle multiple connections and an edit workflow. Feedback points out hardcoded placeholder statistics, invalid HTML from nested buttons in the connection cards, and the potentially accidental removal of SCIM configuration types.
I am having trouble creating individual review comments. Click here to see my feedback.
frontend/web/components/pages/environment-settings/tabs/warehouse-tab/WarehouseConnectionCard.tsx (93-103)
The statistics (lastEventReceived, totalEventsReceived, uniqueEventsCount) and the SQL query string are currently hardcoded with placeholder values (e.g., '19 May 2026', 'SELECT * FROM users'). These should be dynamically populated from the connection data or handled as empty/loading states if the data is not yet available from the API.
frontend/web/components/pages/environment-settings/tabs/warehouse-tab/WarehouseConnectionCard.tsx (136-145)
The Button component (which renders a <button> tag) is nested inside another <button> (the accordion header at line 109). Nesting interactive elements like buttons is invalid HTML and can cause issues with event propagation and accessibility. Consider changing the outer <button> to a <div> and handling the click event there.
frontend/common/types/requests.ts (737-740)
SCIM configuration request types are being removed here. This change is not mentioned in the PR description and seems unrelated to the Snowflake warehouse feature. Please verify if this deletion was intentional or an accidental result of a merge conflict.
|
Fixed 2 other comments are discardable |
| setMenuOpen(!menuOpen) | ||
| }} | ||
| > | ||
| <Icon name='more-vertical' width={16} fill='#656D7B' /> |
There was a problem hiding this comment.
Can you please check if we really need those hardcoded colours ?
docs/if required so people know about the feature.Changes
Frontend
CreateWarehouseConnectionModal) with edit mode supportupdateWarehouseConnectionPATCH mutation in RTK Query serviceside-modal--narrow, 640px)How did you test this code?
npm run typecheck— clean