Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Use type metadata description get/update apis #1876

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions frontend/amundsen_application/api/metadata/v0.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
metadata_blueprint = Blueprint('metadata', __name__, url_prefix='/api/metadata/v0')

TABLE_ENDPOINT = '/table'
TYPE_METADATA_ENDPOINT = '/type_metadata'
FEATURE_ENDPOINT = '/feature'
LAST_INDEXED_ENDPOINT = '/latest_updated_ts'
POPULAR_RESOURCES_ENDPOINT = '/popular_resources'
Expand All @@ -48,6 +49,13 @@ def _get_table_endpoint() -> str:
return metadata_service_base + TABLE_ENDPOINT


def _get_type_metadata_endpoint() -> str:
metadata_service_base = app.config['METADATASERVICE_BASE']
if metadata_service_base is None:
raise Exception('METADATASERVICE_BASE must be configured')
return metadata_service_base + TYPE_METADATA_ENDPOINT


def _get_feature_endpoint() -> str:
metadata_service_base = app.config['METADATASERVICE_BASE']
if metadata_service_base is None:
Expand Down Expand Up @@ -297,6 +305,32 @@ def get_column_description() -> Response:
return make_response(payload, HTTPStatus.INTERNAL_SERVER_ERROR)


@metadata_blueprint.route('/get_type_metadata_description', methods=['GET'])
def get_type_metadata_description() -> Response:
try:
type_metadata_endpoint = _get_type_metadata_endpoint()

type_metadata_key = get_query_param(request.args, 'type_metadata_key')

url = '{0}/{1}/description'.format(type_metadata_endpoint, type_metadata_key)

response = request_metadata(url=url)
status_code = response.status_code

if status_code == HTTPStatus.OK:
message = 'Success'
description = response.json().get('description')
else:
message = 'Get type metadata description failed'
description = None

payload = jsonify({'description': description, 'msg': message})
return make_response(payload, status_code)
except Exception as e:
payload = jsonify({'description': None, 'msg': 'Encountered exception: ' + str(e)})
return make_response(payload, HTTPStatus.INTERNAL_SERVER_ERROR)


@metadata_blueprint.route('/put_table_description', methods=['PUT'])
def put_table_description() -> Response:

Expand Down Expand Up @@ -375,6 +409,46 @@ def _log_put_column_description(*, table_key: str, column_name: str, description
return make_response(payload, HTTPStatus.INTERNAL_SERVER_ERROR)


@metadata_blueprint.route('/put_type_metadata_description', methods=['PUT'])
def put_type_metadata_description() -> Response:

@action_logging
def _log_put_type_metadata_description(*, type_metadata_key: str, description: str, source: str) -> None:
pass # pragma: no cover

try:
args = request.get_json()

type_metadata_endpoint = _get_type_metadata_endpoint()

type_metadata_key = get_query_param(args, 'type_metadata_key')
description = get_query_param(args, 'description')

src = get_query_param(args, 'source')

table_key = get_query_param(args, 'table_key')
table_uri = TableUri.from_uri(table_key)
if not is_table_editable(table_uri.schema, table_uri.table):
return make_response('', HTTPStatus.FORBIDDEN)

url = '{0}/{1}/description'.format(type_metadata_endpoint, type_metadata_key)
_log_put_type_metadata_description(type_metadata_key=type_metadata_key, description=description, source=src)

response = request_metadata(url=url, method='PUT', data=json.dumps({'description': description}))
status_code = response.status_code

if status_code == HTTPStatus.OK:
message = 'Success'
else:
message = 'Update type metadata description failed'

payload = jsonify({'msg': message})
return make_response(payload, status_code)
except Exception as e:
payload = jsonify({'msg': 'Encountered exception: ' + str(e)})
return make_response(payload, HTTPStatus.INTERNAL_SERVER_ERROR)


@metadata_blueprint.route('/tags')
def get_tags() -> Response:
"""
Expand Down
12 changes: 10 additions & 2 deletions frontend/amundsen_application/api/utils/metadata_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@

from dataclasses import dataclass
from marshmallow import EXCLUDE
from typing import Any, Dict, List
from typing import Any, Dict, List, Optional

from amundsen_common.models.dashboard import DashboardSummary, DashboardSummarySchema
from amundsen_common.models.feature import Feature, FeatureSchema
from amundsen_common.models.popular_table import PopularTable, PopularTableSchema
from amundsen_common.models.table import Table, TableSchema
from amundsen_common.models.table import Table, TableSchema, TypeMetadata
from amundsen_application.models.user import load_user, dump_user
from amundsen_application.config import MatchRuleObject
from flask import current_app as app
Expand Down Expand Up @@ -102,6 +102,13 @@ def is_table_editable(schema_name: str, table_name: str, cfg: Any = None) -> boo
return True


def _recursive_set_type_metadata_is_editable(type_metadata: Optional[TypeMetadata], is_editable: bool) -> None:
if type_metadata is not None:
type_metadata['is_editable'] = is_editable
for tm in type_metadata['children']:
_recursive_set_type_metadata_is_editable(tm, is_editable)


def marshall_table_full(table_dict: Dict) -> Dict:
"""
Forms the full version of a table Dict, with additional and sanitized fields
Expand All @@ -128,6 +135,7 @@ def marshall_table_full(table_dict: Dict) -> Dict:
for col in columns:
dkunitsk marked this conversation as resolved.
Show resolved Hide resolved
# Set editable state
col['is_editable'] = is_editable
_recursive_set_type_metadata_is_editable(col['type_metadata'], is_editable)
# If order is provided, we sort the column based on the pre-defined order
if app.config['COLUMN_STAT_ORDER']:
# the stat_type isn't defined in COLUMN_STAT_ORDER, we just use the max index for sorting
Expand Down
28 changes: 8 additions & 20 deletions frontend/amundsen_application/static/.betterer.results
Original file line number Diff line number Diff line change
Expand Up @@ -65,22 +65,11 @@ exports[`eslint`] = {
[118, 19, 20, "Must use destructuring state assignment", "3067028466"],
[126, 10, 137, "Visible, non-interactive elements with click handlers must have at least one keyboard listener.", "3965651236"]
],
"js/components/EditableText/index.tsx:2168483193": [
"js/components/EditableText/index.tsx:1961540365": [
[51, 2, 21, "textAreaRef should be placed after componentDidUpdate", "3072052035"],
[72, 6, 13, "Do not use setState in componentDidUpdate", "57229240"],
[80, 10, 25, "Must use destructuring props assignment", "793704523"],
[81, 8, 25, "Must use destructuring props assignment", "793704523"],
[83, 32, 16, "Must use destructuring state assignment", "3998965439"],
[83, 53, 21, "Must use destructuring state assignment", "1159122654"],
[85, 6, 13, "Do not use setState in componentDidUpdate", "57229240"],
[90, 4, 22, "Must use destructuring props assignment", "2225424112"],
[94, 4, 22, "Must use destructuring props assignment", "2225424112"],
[98, 27, 23, "Must use destructuring props assignment", "2982501243"],
[101, 23, 23, "Must use destructuring props assignment", "2982501243"],
[109, 6, 22, "Must use destructuring props assignment", "2225424112"],
[116, 4, 24, "Must use destructuring props assignment", "3049746099"],
[134, 19, 20, "Script URL is a form of eval.", "3373049033"],
[146, 8, 207, "A control must be associated with a text label.", "2914986446"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

[78, 6, 13, "Do not use setState in componentDidUpdate", "57229240"],
[95, 6, 13, "Do not use setState in componentDidUpdate", "57229240"],
[148, 19, 20, "Script URL is a form of eval.", "3373049033"]
],
"js/components/EntityCard/EntityCardSection/index.tsx:474926744": [
[25, 2, 55, "editButton should be placed after constructor", "2479426463"],
Expand Down Expand Up @@ -444,9 +433,8 @@ exports[`eslint`] = {
[19, 2, 8, "Assignment to function parameter \'resource\'.", "2131237679"],
[20, 2, 248, "Expected a default case.", "1034339850"]
],
"js/ducks/tableMetadata/api/v0.ts:4194065289": [
[80, 8, 23, "Use object destructuring.", "1142306891"],
[139, 23, -4311, "Expected to return a value at the end of arrow function.", "5381"]
"js/ducks/tableMetadata/api/v0.ts:3505354085": [
[140, 23, -4358, "Expected to return a value at the end of arrow function.", "5381"]
],
"js/ducks/tableMetadata/owners/index.spec.ts:655040122": [
[15, 0, 91, "\`../reducer\` import should occur before import of \`./reducer\`", "2216296793"],
Expand All @@ -455,8 +443,8 @@ exports[`eslint`] = {
"js/ducks/tableMetadata/owners/sagas.ts:3725515638": [
[7, 0, 69, "\`../types\` import should occur before import of \`./reducer\`", "3326352266"]
],
"js/ducks/tableMetadata/reducer.ts:1012064960": [
[416, 6, 93, "Unexpected lexical declaration in case block.", "4098864482"]
"js/ducks/tableMetadata/reducer.ts:3842494077": [
[480, 6, 93, "Unexpected lexical declaration in case block.", "4098864482"]
],
"js/ducks/tags/api/v0.ts:2781466514": [
[21, 16, -605, "Expected to return a value at the end of function \'getResourceTags\'.", "5381"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,53 +68,67 @@ class EditableText extends React.Component<
}

componentDidUpdate(prevProps) {
const { value, isEditing, refreshValue } = this.props;
if (prevProps.value !== value) {
this.setState({ value });
const { value: stateValue, isDisabled } = this.state;
const {
value: propValue,
isEditing,
refreshValue,
getLatestValue,
} = this.props;
if (prevProps.value !== propValue) {
this.setState({ value: propValue });
} else if (isEditing && !prevProps.isEditing) {
const textArea = this.textAreaRef.current;
if (textArea) {
autosize(textArea);
textArea.focus();
}

if (this.props.getLatestValue) {
this.props.getLatestValue();
if (getLatestValue) {
getLatestValue();
}
} else if (refreshValue !== this.state.value && !this.state.isDisabled) {
} else if (
(refreshValue || stateValue) &&
refreshValue !== stateValue &&
!isDisabled
) {
// disable the component if a refresh is needed
this.setState({ isDisabled: true });
}
}

exitEditMode = () => {
this.props.setEditMode?.(false);
const { setEditMode } = this.props;
setEditMode?.(false);
};

enterEditMode = () => {
this.props.setEditMode?.(true);
const { setEditMode } = this.props;
setEditMode?.(true);
};

refreshText = () => {
this.setState({ value: this.props.refreshValue, isDisabled: false });
const { refreshValue } = this.props;
this.setState({ value: refreshValue, isDisabled: false });
const textArea = this.textAreaRef.current;
if (textArea) {
textArea.value = this.props.refreshValue;
textArea.value = refreshValue;
autosize.update(textArea);
}
};

updateText = () => {
const { setEditMode, onSubmitValue } = this.props;
const newValue = this.textAreaRef.current.value;
const onSuccessCallback = () => {
this.props.setEditMode?.(false);
setEditMode?.(false);
this.setState({ value: newValue });
};
const onFailureCallback = () => {
this.exitEditMode();
};

this.props.onSubmitValue?.(newValue, onSuccessCallback, onFailureCallback);
onSubmitValue?.(newValue, onSuccessCallback, onFailureCallback);
};

render() {
Expand Down Expand Up @@ -151,6 +165,7 @@ class EditableText extends React.Component<
ref={this.textAreaRef}
defaultValue={value}
disabled={isDisabled}
aria-label="Editable text area"
/>
<div className="editable-textarea-controls">
{isDisabled && (
Expand Down
6 changes: 4 additions & 2 deletions frontend/amundsen_application/static/js/ducks/rootSaga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,12 @@ import { updateTableOwnerWatcher } from './tableMetadata/owners/sagas';
import {
getTableDataWatcher,
getColumnDescriptionWatcher,
getTypeMetadataDescriptionWatcher,
getPreviewDataWatcher,
getTableDescriptionWatcher,
getTableQualityChecksWatcher,
updateColumnDescriptionWatcher,
updateTypeMetadataDescriptionWatcher,
updateTableDescriptionWatcher,
} from './tableMetadata/sagas';

Expand Down Expand Up @@ -132,13 +134,13 @@ export default function* rootSaga() {
updateResourceTagsWatcher(),
// TableDetail
getTableDataWatcher(),

getColumnDescriptionWatcher(),

getTypeMetadataDescriptionWatcher(),
getPreviewDataWatcher(),
getTableDescriptionWatcher(),
getTableQualityChecksWatcher(),
updateColumnDescriptionWatcher(),
updateTypeMetadataDescriptionWatcher(),
updateTableDescriptionWatcher(),
updateTableOwnerWatcher(),
// LastIndexed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,33 @@ export function getColumnCount(columns: TableColumn[]) {
columns.length
);
}

/**
* Given a type metadata key, returns the associated type metadata object
*/
export function getTypeMetadataFromKey(
tmKey: string,
tableData: TableMetadata
) {
const tmNamePath = tmKey.replace(tableData.key + '/', '');

const [
columnName,
typeConstant, // eslint-disable-line @typescript-eslint/no-unused-vars
topLevelTmName, // eslint-disable-line @typescript-eslint/no-unused-vars
Comment on lines +130 to +131
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can do:

const [ columnName, _, _, ...tmNames] ...

Copy link
Contributor Author

@kristenarmes kristenarmes Jun 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I was getting the error Cannot redeclare block-scoped variable '_' since there are two, but in any case I seem to remember from a previous PR where I had I tried the underscore for one unused var, the checks still required the no unused var disable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, makes sense to complain with two of those.
Whatever works will be fine.

...tmNames
] = tmNamePath.split('/');

const column = tableData.columns.find((column) => column.name === columnName);

let typeMetadata = column?.type_metadata;
// Find the TypeMetadata object at each level corresponding to its name from the key path
tmNames.forEach((nextLevelTmName) => {
const nextTmObject = typeMetadata?.children?.find(
(child) => child.name === nextLevelTmName
);
typeMetadata = nextTmObject;
});

return typeMetadata;
}
Loading