laserninja commented on code in PR #12900:
URL: https://github.com/apache/gravitino/pull/12900#discussion_r4297082303
##########
web-v2/web/src/lib/store/metalakes/index.js:
##########
@@ -2364,6 +2535,25 @@ export const appMetalakesSlice = createSlice({
currentEntityPolicies: []
},
reducers: {
+ setActiveCatalogEntity(state, action) {
+ const key = catalogEntityKey(action.payload)
+ const schemaKey = action.payload.schema ?
semanticModelSchemaKey(action.payload) : null
+ if (state.activeSemanticModelSchema !== schemaKey) state.semanticModels
= []
+ state.activeSemanticModelSchema = schemaKey
+ if (state.activeCatalogEntity === key) return
+ state.activeCatalogEntity = key
+ state.activatedDetails = null
Review Comment:
Fixed the version-route save/refresh path in 17709890c. The current version
now travels from the parent model dialog through updateModel into
getModelDetails, so the refresh matches the active version identity without
relaxing the guard for other models. Tests cover comment/property updates from
model and version URLs, plus delayed saves after switching model or namespace.
Chromium confirms that both the comment and property shown on the version route
update after saving. I also added the positive-path table/fileset/topic
detail-loading coverage mentioned in the review summary, including table
columns and partition/distribution properties.
##########
web-v2/web/src/app/catalogs/rightContent/SemanticMetadataEditor.js:
##########
@@ -0,0 +1,390 @@
+/*
+ * 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 { useEffect, useRef, useState } from 'react'
+import { Alert, Button, Form, Input, Modal, Select, Space, Tabs, theme } from
'antd'
+import { isEqual } from 'lodash-es'
+import { useAppDispatch } from '@/lib/hooks/useStore'
+import { updateSemanticModel } from '@/lib/store/metalakes'
+import { semanticDataTypes, validateSemanticModelDefinition } from
'@/lib/utils/semanticModel'
+import {
+ getSemanticMetadataEntity,
+ semanticMetadataFormValues,
+ updateSemanticMetadata
+} from '@/lib/utils/semanticMetadata'
+
+export default function SemanticMetadataEditor(props) {
+ try {
+ getSemanticMetadataEntity(props.definition, props.target)
+ } catch (error) {
+ return (
+ <Modal
+ open
+ title='Metadata unavailable'
+ onCancel={props.onClose}
+ footer={<Button onClick={props.onClose}>Close</Button>}
+ >
+ <Alert type='error' showIcon message={error.message} />
+ </Modal>
+ )
+ }
+
+ return <SemanticMetadataForm {...props} />
+}
+
+function SemanticMetadataForm({ definition, target, metalake, catalog, schema,
semanticModel, onClose, onSaved }) {
+ const { token } = theme.useToken()
+ const dispatch = useAppDispatch()
+ const [form] = Form.useForm()
+ const [saving, setSaving] = useState(false)
+ const [errors, setErrors] = useState([])
+ const [section, setSection] = useState(target.kind === 'model' ? 'context' :
target.section || 'basic')
+ const [focusField, setFocusField] = useState(null)
+ const [confirmDiscard, setConfirmDiscard] = useState(false)
+ const errorRef = useRef(null)
+ const expressionRef = useRef(null)
+
+ useEffect(() => {
+ if (!focusField) return
+
+ // Tabs reveal their panel after the active key changes; focus once it is
visible.
+ const frame = requestAnimationFrame(() => {
+ form.scrollToField(focusField, { block: 'nearest' })
+ const input = form.getFieldInstance(focusField)
+ if (input?.focus) input.focus()
+ else if (focusField[0] === 'dialects') expressionRef.current?.focus()
+ setFocusField(null)
+ })
+
+ return () => cancelAnimationFrame(frame)
+ }, [section, focusField, form])
+
+ useEffect(() => {
+ if (errors.length) errorRef.current?.focus()
+ }, [errors])
+
+ // Keep the editing snapshot stable through list/detail refreshes while the
dialog is open.
+ const [original] = useState(definition)
+ const entity = getSemanticMetadataEntity(original, target)
+ const [initial] = useState(() => semanticMetadataFormValues(entity))
+ const structuredContextInitialized = useRef(initial.contextFormat ===
'structured')
+ const contextFormat = Form.useWatch('contextFormat', form) ||
initial.contextFormat
+ const hasExpression = ['field', 'metric'].includes(target.kind)
+ const title = target.kind === 'model' ? 'Edit Model AI Context' : `Edit
${target.kind} ${target.name}`
+
+ const close = () => {
+ if (saving) return
+ if (isEqual(initial, form.getFieldsValue(true))) onClose()
+ else setConfirmDiscard(true)
+ }
+
+ const save = async () => {
+ try {
+ await form.validateFields()
+ } catch (failure) {
+ const name = failure.errorFields?.[0]?.name
+ if (name) {
+ setSection(
+ name[0] === 'dialects'
+ ? 'expressions'
+ : ['contextFormat', 'contextText', 'instructions', 'synonyms',
'examples'].includes(name[0])
+ ? 'context'
+ : 'basic'
+ )
+ setFocusField(name)
+ }
+
+ return
+ }
+ const updated = updateSemanticMetadata(original, target,
form.getFieldsValue(true))
+ const problems = validateSemanticModelDefinition(updated)
+ setErrors(problems)
+ if (problems.length) return
+ if (isEqual(original, updated)) {
+ onClose()
+
+ return
+ }
+ setSaving(true)
+ try {
+ const result = await dispatch(
+ updateSemanticModel({
+ metalake,
+ catalog,
+ schema,
+ semanticModel,
+ data: { updates: [{ '@type': 'replaceDefinition', definition:
updated }] }
Review Comment:
Added a shared pre-save check in 17709890c for both definition editors.
Before replaceDefinition, the update thunk re-reads the current model and
compares its definition with the editor snapshot; a mismatch or failed
verification prevents the write. Both dialogs retain the draft and show
recovery guidance. Tests and Chromium checks cover concurrent edits in both
dialogs, unchanged definitions, and failed verification reads. This remains a
best-effort check: a writer can still change the definition between the GET and
PUT, and closing that race requires a server-side expected-version token.
##########
web-v2/web/src/lib/utils/semanticMetadata.js:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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 { cloneDeep, isEqual } from 'lodash-es'
+
+const editableProperties = {
+ model: [],
+ dataset: ['description'],
+ field: ['label', 'description', 'datatype'],
Review Comment:
Added a clearable Time dimension selector to the field editor Basic
information tab in 17709890c. Yes and No write the corresponding boolean;
leaving it unset preserves an absent dimension, and explicitly clearing it
removes isTime and an otherwise empty dimension object. Additional dimension
metadata is preserved. Utility/component tests cover the states, and Chromium
verifies the field edit and saved definition.
##########
web-v2/web/src/lib/utils/semanticMetadata.js:
##########
@@ -0,0 +1,97 @@
+/*
+ * 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 { cloneDeep, isEqual } from 'lodash-es'
+
+const editableProperties = {
+ model: [],
+ dataset: ['description'],
+ field: ['label', 'description', 'datatype'],
+ metric: ['description', 'datatype'],
+ relationship: ['from', 'to', 'fromColumns', 'toColumns']
+}
+
+const missingMetadataMessage = 'The selected metadata no longer exists. Reload
the model and try again.'
+
+/** Finds the selected entity without flattening dataset-scoped field names. */
+export const getSemanticMetadataEntity = (definition, target) => {
+ const entity =
+ target.kind === 'model'
+ ? definition
+ : target.kind === 'field'
+ ? definition?.datasets
+ ?.find(dataset => dataset.name === target.datasetName)
+ ?.fields?.find(field => field.name === target.name)
+ : definition?.[`${target.kind}s`]?.find(item => item.name ===
target.name)
+ if (!entity) throw new Error(missingMetadataMessage)
+
+ return entity
+}
+
+/** Supplies form values while keeping the original wire representation
separate. */
+export const semanticMetadataFormValues = entity => {
+ if (!entity) throw new Error(missingMetadataMessage)
+
+ return {
+ ...cloneDeep(entity),
+ dialects: cloneDeep(entity.expression?.dialects || []),
+ contextFormat: typeof entity.aiContext === 'string' ? 'text' :
'structured',
+ contextText: typeof entity.aiContext === 'string' ? entity.aiContext : '',
+ instructions: typeof entity.aiContext === 'string' ? entity.aiContext :
entity.aiContext?.instructions || '',
+ synonyms: cloneDeep(entity.aiContext?.synonyms || []),
+ examples: cloneDeep(entity.aiContext?.examples || [])
+ }
+}
+
+/** Applies only edited form values, preserving every other entity and
extension in the definition. */
+export const updateSemanticMetadata = (definition, target, values) => {
+ const updated = cloneDeep(definition)
+ const entity = getSemanticMetadataEntity(updated, target)
+ const initial = semanticMetadataFormValues(entity)
+ for (const key of editableProperties[target.kind]) {
+ const value = key === 'datatype' && values[key] === '' ? null : values[key]
+ if (!isEqual(initial[key], value)) entity[key] = value ?? null
+ }
+ if (['field', 'metric'].includes(target.kind) && !isEqual(initial.dialects,
values.dialects)) {
+ entity.expression = { ...entity.expression, dialects:
cloneDeep(values.dialects) }
+ }
+
+ if (values.contextFormat === 'text') {
+ if (initial.contextFormat !== 'text' || initial.contextText !==
values.contextText) {
+ // Existing structured guidance stays structured so its extra keys
cannot be discarded.
+ if (entity.aiContext && typeof entity.aiContext === 'object') {
+ entity.aiContext.instructions = values.contextText
+ } else {
+ entity.aiContext = values.contextText
Review Comment:
Fixed in 17709890c. Clearing optional labels, descriptions, or text guidance
now removes the key instead of persisting an empty string. Clearing structured
guidance also removes empty known fields and drops the context when nothing
remains, including when synonym/example lists were already empty. Unknown
context keys and vendor extensions stay intact. Tests cover empty/whitespace
input and structured cleanup; Chromium confirms the cleared keys are absent
from the saved definition.
##########
web-v2/web/src/app/catalogs/page.js:
##########
@@ -332,33 +357,50 @@ const CatalogsListPage = () => {
}
}
if (func) {
- store.functions.length === 0 && (await dispatch(fetchFunctions({
init: true, metalake, catalog, schema })))
- await dispatch(setActivatedDetailsLoading(true))
- await dispatch(getFunctionDetails({ init: true, metalake, catalog,
schema, functionName: func }))
- await dispatch(setActivatedDetailsLoading(false))
+ store.functions.length === 0 && (await load(fetchFunctions({ init:
true, metalake, catalog, schema })))
+ await load(setActivatedDetailsLoading(true))
+ await load(getFunctionDetails({ init: true, metalake, catalog,
schema, functionName: func }))
+ await load(setActivatedDetailsLoading(false))
}
if (view) {
- store.views.length === 0 && (await dispatch(fetchViews({ init:
true, metalake, catalog, schema })))
- await dispatch(setActivatedDetailsLoading(true))
- await dispatch(getViewDetails({ init: true, metalake, catalog,
schema, view }))
- await dispatch(setActivatedDetailsLoading(false))
+ store.views.length === 0 && (await load(fetchViews({ init: true,
metalake, catalog, schema })))
+ await load(setActivatedDetailsLoading(true))
+ await load(getViewDetails({ init: true, metalake, catalog, schema,
view }))
+ await load(setActivatedDetailsLoading(false))
}
- }
- if (paramsSize === 6 && version) {
- if (!store.catalogs.length) {
- await dispatch(fetchCatalogs({ metalake }))
- await dispatch(fetchSchemas({ metalake, catalog, catalogType }))
- await dispatch(fetchModels({ init: true, page: 'schemas',
metalake, catalog, schema }))
+ if (semanticModel) {
+ if (canceled) return
+ store.semanticModels.length === 0 &&
+ (await load(fetchSemanticModels({ init: true, metalake, catalog,
schema })))
+ if (canceled) return
+ load(resetActivatedDetails())
+ load(setActivatedDetailsLoading(true))
+
+ const detailRequest = load(
+ getSemanticModelDetails({ init: true, metalake, catalog, schema,
semanticModel })
+ )
+ await detailRequest
+ if (canceled) return
+ load(setActivatedDetailsLoading(false))
+ load(
+ getCurrentEntityTags({
+ init: true,
+ metalake,
+ metadataObjectType: 'semantic_model',
Review Comment:
This was an omission. In 17709890c the Semantic Model route loads policies
using semantic_model and the full model name, and Overview displays the policy
count with the existing policy component in its popover. Added navigation and
component tests for the metadata identity and policy display, and verified the
associated policy in Chromium.
##########
web-v2/web/src/app/catalogs/rightContent/entitiesContent/SchemaDetailsPage.js:
##########
@@ -804,6 +913,62 @@ export default function SchemaDetailsPage() {
components={viewComponents}
/>
</>
+ ) : tabKey === 'Semantic Models' ? (
+ <>
+ <Flex justify='flex-end' className='mb-4' gap='middle' wrap>
+ <Search
+ className='w-full sm:w-64'
+ name='searchSemanticModelInput'
+ placeholder='Search...'
+ value={search}
+ onChange={onSearchTable}
+ />
+ <Space wrap>
+ <Button onClick={() => setOpenImportSemanticModel(true)}>Import
OSSIE</Button>
+ <Button
+ data-refer='create-semantic-model-btn'
+ type='primary'
+ icon={<PlusOutlined />}
+ onClick={() => {
+ setEditSemanticModel('')
+ setOpenSemanticModel(true)
+ }}
+ >
+ Create Semantic Model
+ </Button>
+ </Space>
+ </Flex>
+ <Spin spinning={store.tableLoading}>
Review Comment:
Fixed in 17709890c by deriving list loading from the current schema request
record and its in-flight list request ID. The tab now shows loading while its
model list is pending, with the existing operation spinner still in place. The
list loading state clears on success, failure, or abort. Component tests cover
all three outcomes and an unrelated schema request; a delayed API response in
Chromium now shows the spinner until the list arrives.
--
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]
