Copilot commented on code in PR #12880: URL: https://github.com/apache/gravitino/pull/12880#discussion_r3931397510
########## web-v2/web/src/lib/utils/axios/unsupportedOperation.js: ########## @@ -0,0 +1,24 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +const UNSUPPORTED_OPERATION_CODE = 1006 + +/** Returns whether an HTTP client error represents an unsupported server operation. */ +export const isUnsupportedOperationError = error => + error?.response?.data?.code === UNSUPPORTED_OPERATION_CODE || error?.response?.status === 501 Review Comment: `isUnsupportedOperationError` currently returns true whenever the application error code is 1006, regardless of HTTP status. In this PR, some 409 Conflict responses (e.g., unmodifiable statistics) intentionally reuse code 1006 for client compatibility, so this helper would misclassify conflicts as “unsupported capability” if it gets reused beyond the views calls. ########## web-v2/web/src/lib/utils/axios/unsupportedOperation.test.js: ########## @@ -0,0 +1,39 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { describe, expect, it } from 'vitest' +import { isUnsupportedOperationError } from '@/lib/utils/axios/unsupportedOperation' + +describe('isUnsupportedOperationError', () => { + it('recognizes HTTP 501 as an unsupported operation', () => { + expect(isUnsupportedOperationError({ response: { status: 501 } })).toBe(true) + }) + + it('recognizes the legacy HTTP 405 response by its application error code', () => { + expect(isUnsupportedOperationError({ response: { status: 405, data: { code: 1006 } } })).toBe(true) + }) + + it('does not hide an actual HTTP method mismatch', () => { + expect(isUnsupportedOperationError({ response: { status: 405, data: { code: 1000 } } })).toBe(false) + }) Review Comment: The tests don’t cover the case where the server returns HTTP 409 with application code 1006 (used for `UnmodifiableStatisticException` in this PR). Adding a regression test helps ensure `isUnsupportedOperationError` doesn’t accidentally start treating conflicts as unsupported capabilities again. ########## web-v2/web/src/lib/store/metalakes/index.js: ########## @@ -2250,8 +2251,8 @@ export const fetchViews = createAsyncThunk( const [err, res] = await to(getViewsApi({ metalake, catalog, schema }, { errorMessageMode: 'none' })) if (err || !res) { - // Catalog doesn't support views (HTTP 405) — return empty views silently - if (err?.response?.status === 405) { + // Catalog doesn't support views (HTTP 501) — return empty views silently Review Comment: This comment says the unsupported-views case is “HTTP 501”, but the fallback helper also treats the legacy HTTP 405 + code 1006 payload as unsupported. Please update the comment so it matches the actual behavior and avoids misleading future maintainers. This issue also appears on line 2308 of the same file. -- 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]
