Copilot commented on code in PR #11479:
URL: https://github.com/apache/gravitino/pull/11479#discussion_r3371188551
##########
web-v2/web/src/app/catalogs/rightContent/CreateCatalogDialog.js:
##########
@@ -77,6 +77,13 @@ export default function CreateCatalogDialog({ ...props }) {
const dispatch = useAppDispatch()
const isShowTestConnect = ['fileset', 'model'].includes(catalogType) ||
currentProvider === 'lakehouse-generic'
+ const isRequiredField = prop => {
+ return (
+ prop.required ||
+ (prop.key === 'warehouse' && currentProvider === 'lakehouse-iceberg' &&
['hive', 'jdbc'].includes(catalogBackend))
+ )
+ }
Review Comment:
This PR is scoped (per title/linked issue) to fixing Iceberg bucketed table
editing, but it also changes CreateCatalogDialog behavior (new conditional
required/visibility logic for the `warehouse` field). If this is intentional,
please update the PR description to mention it; otherwise consider moving this
change to a separate PR to keep the fix focused and easier to review/rollback.
##########
web-v2/web/src/app/catalogs/rightContent/CreateCatalogDialog.js:
##########
@@ -113,7 +120,7 @@ export default function CreateCatalogDialog({ ...props }) {
case 'authentication.type':
return !authType || (hide && hide.includes(authType))
default:
- return !(!editCatalog || ['region', 'location'].includes(key) ||
required)
+ return !(!editCatalog || ['region', 'location'].includes(key) ||
isRequiredField(prop))
Review Comment:
`isHidden` now uses `isRequiredField(prop)` instead of the destructured
`required` value. As a result, the `required` destructuring in `isHidden`
becomes unused and will likely fail the `eslint src --max-warnings=0` lint step.
--
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]
