Skip to content

Commit

Permalink
Issue 147/multi doc pathways (#381)
Browse files Browse the repository at this point in the history
* This adds "programmatic descriptions" into the frontend.

Known issues:
- is_editable property not respecected

Added NonEditable Section

Now no longer need to sort in index page.
Uses the display title for title now.
Added currently untested code in the api for programmatic display

programmatic descriptions no longer needs to be returned by metadata service for backwards compatibility
Tests added for the programmatic display component

Caught a bug where display_title was not being set if configuration wasn't set.
change source_id to source
Make code more robust
Rebasing with the upstream changes that have been made.
adding documentation
removing the comment in the config.py class

Fixing upstream merge conflicts.

Update docs/flask_config.md

Co-Authored-By: jornh <[email protected]>

Removing non editable section!

readOnly is now an optional Property
Added in a programmatic header and <hr> per design doc

adding test for button rendering

Adding in convertText function
Changing SENTENCE_CASE -> Upper Upper. Need to confirm that this is ok, otherwise can create a new case

Reverting SentenceCase
Creating PascalCase
Removing custom title

Moving convert text to EditableSection. Applying by default

* removing pascal case

* fixing doc

* Update amundsen_application/api/utils/metadata_utils.py

Co-Authored-By: Tamika Tannis <[email protected]>

* Update amundsen_application/api/utils/metadata_utils.py

Co-Authored-By: Tamika Tannis <[email protected]>

* cleaning up

* Fixing unit test to have static method

* changing to edit-button

* Moving tests to test_metadata_utils

* Fixing lint

* updating the sample image

Co-authored-by: Tamika Tannis <[email protected]>
  • Loading branch information
samshuster and ttannis authored Mar 16, 2020
1 parent c762a8f commit ef49b88
Show file tree
Hide file tree
Showing 16 changed files with 299 additions and 15 deletions.
31 changes: 30 additions & 1 deletion amundsen_application/api/utils/metadata_utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from typing import Any, Dict
import logging

from typing import Any, Dict, List

from amundsen_common.models.popular_table import PopularTable, PopularTableSchema
from amundsen_common.models.table import Table, TableSchema
Expand Down Expand Up @@ -63,9 +65,36 @@ def marshall_table_full(table_dict: Dict) -> Dict:
results['key'] = f'{table.database}://{table.cluster}.{table.schema}/{ table.name}'
# Temp code to make 'partition_key' and 'partition_value' part of the table
results['partition'] = _get_partition_data(results['watermarks'])

# We follow same style as column stat order for arranging the programmatic descriptions
prog_descriptions = results['programmatic_descriptions']
if prog_descriptions:
_update_prog_descriptions(prog_descriptions)

return results


def _update_prog_descriptions(prog_descriptions: List) -> None:
# We want to make sure there is a display title that is just source
for desc in prog_descriptions:
source = desc.get('source')
if not source:
logging.warning("no source found in: " + str(desc))
prog_display_config = app.config['PROGRAMMATIC_DISPLAY']
if prog_display_config and prog_descriptions:
# If config is defined for programmatic disply we look to see what configuration is being used
prog_descriptions.sort(key=lambda x: _sort_prog_descriptions(prog_display_config, x))


def _sort_prog_descriptions(base_config: Dict, prog_description: Dict) -> int:
default_order = len(base_config)
prog_description_source = prog_description.get('source')
config_dict = base_config.get(prog_description_source)
if config_dict:
return config_dict.get('display_order', default_order)
return default_order


def _map_user_object_to_schema(u: Dict) -> Dict:
return dump_user(load_user(u))

Expand Down
9 changes: 9 additions & 0 deletions amundsen_application/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ class Config:
# Max issues to display at a time
ISSUE_TRACKER_MAX_RESULTS = None # type: int

# Programmatic Description configuration. Please see docs/flask_config.md
PROGRAMMATIC_DISPLAY = None # type: Optional[Dict]

# If specified, will be used to generate headers for service-to-service communication
# Please note that if specified, this will ignore following config properties:
# 1. METADATASERVICE_REQUEST_HEADERS
Expand Down Expand Up @@ -112,6 +115,12 @@ class TestConfig(LocalConfig):
ISSUE_TRACKER_CLIENT_ENABLED = True
ISSUE_TRACKER_MAX_RESULTS = 3

PROGRAMMATIC_DISPLAY = {
"a_1": {
"display_order": 0
}
}


class TestNotificationsDisabledConfig(LocalConfig):
AUTH_USER_METHOD = get_test_user
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import './styles.scss';

export interface EditableSectionProps {
title: string;
readOnly?: boolean;
}

interface EditableSectionState {
Expand Down Expand Up @@ -32,6 +33,10 @@ export class EditableSection extends React.Component<EditableSectionProps, Edita
this.setState({ isEditing: !this.state.isEditing });
};

static convertText(str: string): string {
return str.split(new RegExp('[\\s+_]')).map(x => x.charAt(0).toUpperCase() + x.slice(1).toLowerCase()).join(" ");
}

render() {
const childrenWithProps = React.Children.map(this.props.children, child => {
if (!React.isValidElement(child)) {
Expand All @@ -46,10 +51,13 @@ export class EditableSection extends React.Component<EditableSectionProps, Edita
return (
<section className="editable-section">
<div className="section-title title-3">
{ this.props.title }
<button className={"btn btn-flat-icon edit-button" + (this.state.isEditing? " active": "")} onClick={ this.toggleEdit }>
<img className={"icon icon-small icon-edit" + (this.state.isEditing? " icon-color" : "")} />
</button>
{ EditableSection.convertText(this.props.title) }
{
!this.props.readOnly &&
<button className={"btn btn-flat-icon edit-button" + (this.state.isEditing? " active": "")} onClick={ this.toggleEdit }>
<img className={"icon icon-small icon-edit" + (this.state.isEditing? " icon-color" : "")} />
</button>
}
</div>
{ childrenWithProps }
</section>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
import * as React from 'react';
import { shallow } from 'enzyme';

import { EditableSection, EditableSectionProps } from '../';
import {EditableSection, EditableSectionProps} from '../';
import TagInput from 'components/Tags/TagInput';


describe("EditableSection", () => {
const setup = (propOverrides?: Partial<EditableSectionProps>, children?) => {
const props = {
title: "defaultTitle",
readOnly: false,
...propOverrides,
};
const wrapper = shallow<EditableSection>(<EditableSection {...props} >{ children }</EditableSection>)
return { wrapper, props };
};

describe("setEditMode", () => {
const { wrapper, props } = setup();

Expand Down Expand Up @@ -49,7 +50,7 @@ describe("EditableSection", () => {
const { wrapper, props } = setup({ title: customTitle }, <TagInput/>);

it("sets the title from a prop", () => {
expect(wrapper.find(".section-title").text()).toBe(customTitle);
expect(wrapper.find(".section-title").text()).toBe("Custom Title");
});

it("renders children with additional props", () => {
Expand All @@ -65,5 +66,18 @@ describe("EditableSection", () => {
const { wrapper } = setup(null, child);
expect(wrapper.childAt(1).text()).toBe(child);
});

it("renders button when readOnly=false", () => {
expect(wrapper.find(".edit-button").length).toEqual(1);
});

it("renders does not add button when readOnly=true", () => {
const { wrapper } = setup({readOnly: true}, <TagInput/>);
expect(wrapper.find(".edit-button").length).toEqual(0);
});

it('renders modifies title to have no underscores', () => {
expect(EditableSection.convertText("testing_a123_b456 c789")).toEqual("Testing A123 B456 C789")
})
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ describe('ExploreButton', () => {
table_readers: [],
source: { source: '', source_type: '' },
watermarks: [],
programmatic_descriptions: [],
...tableDataOverrides,
},
};
Expand Down
25 changes: 23 additions & 2 deletions amundsen_application/static/js/components/TableDetail/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,18 @@ import TableIssues from 'components/TableDetail/TableIssues';
import WatermarkLabel from 'components/TableDetail/WatermarkLabel';
import WriterLink from 'components/TableDetail/WriterLink';
import TagInput from 'components/Tags/TagInput';
import { TableMetadata } from 'interfaces/TableMetadata';
import {TableMetadata} from 'interfaces/TableMetadata';

import { EditableSection } from 'components/TableDetail/EditableSection';

import { getDatabaseIconClass, issueTrackingEnabled, notificationsEnabled } from 'config/config-utils';

import { ResourceType } from 'interfaces/Resources';
import { formatDateTimeShort } from 'utils/dateUtils';

import './styles';
import RequestDescriptionText from './RequestDescriptionText';
import RequestMetadataForm from './RequestMetadataForm';
import EditableText from "components/common/EditableText";

export interface StateFromProps {
isLoading: boolean;
Expand Down Expand Up @@ -211,6 +211,27 @@ class TableDetail extends React.Component<TableDetailProps & RouteComponentProps
</EditableSection>
</section>
</section>
{data.programmatic_descriptions.length > 0 &&
<>
<div className="programmatic-title title-4">Read-Only information, Auto-Generated.</div>
<hr className="programmatic-hr hr1"/>
</>
}
{
data.programmatic_descriptions
.map(d =>
<section key={d.source} className="column-layout-2">
<EditableSection title={d.source} readOnly={true}>
<EditableText
maxLength={999999}
value={d.text}
editable={false}
onSubmitValue={null}
/>
</EditableSection>
</section>
)
}
</section>
<section className="right-panel">
<ColumnList columns={ data.columns }/>
Expand Down
11 changes: 11 additions & 0 deletions amundsen_application/static/js/components/TableDetail/styles.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@
margin: 40px 0 8px;
}

.programmatic-title {
color: $text-primary;
margin: 30px 0 10px;
font-weight: $font-weight-body-bold;
}

.programmatic-hr {
border: 1px solid $gray15;
margin: 10px 0 4px;
}

.editable-text {
font-size: 16px;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import './styles.scss';
export enum CaseType {
LOWER_CASE = 'lowerCase',
SENTENCE_CASE = 'sentenceCase',
UPPER_CASE = 'upperCase',
UPPER_CASE = 'upperCase'
}

export interface FlagProps {
Expand Down
1 change: 1 addition & 0 deletions amundsen_application/static/js/config/tests/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ describe('generateExploreUrl', () => {
table_readers: [],
source: { source: '', source_type: '' },
watermarks: [],
programmatic_descriptions: []
};


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ export const initialTableDataState: TableMetadata = {
table_readers: [],
source: { source: '', source_type: '' },
watermarks: [],
programmatic_descriptions: []
};

export const initialState: TableMetadataReducerState = {
Expand Down
1 change: 1 addition & 0 deletions amundsen_application/static/js/fixtures/globalState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ const globalState: GlobalState = {
table_readers: [],
source: { source: '', source_type: '' },
watermarks: [],
programmatic_descriptions: []
},
tableOwners: {
isLoading: true,
Expand Down
6 changes: 6 additions & 0 deletions amundsen_application/static/js/interfaces/TableMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ export interface TableOwners {
owners: User[];
}

export interface ProgrammaticDescription {
source: string;
text: string;
}

export interface TableMetadata {
badges: Badge[];
cluster: string;
Expand All @@ -85,6 +90,7 @@ export interface TableMetadata {
table_readers: TableReader[];
source: TableSource;
watermarks: Watermark[];
programmatic_descriptions: ProgrammaticDescription[];
}

export interface UpdateOwnerPayload {
Expand Down
33 changes: 33 additions & 0 deletions docs/flask_config.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,36 @@ Here are the settings and what they should be set to
ISSUE_TRACKER_MAX_RESULTS = None # type: int (Max issues to display at a time)

```

## Programmatic Descriptions
Amundsen supports configuring other mark down supported non-editable description boxes on the table page.
This can be useful if you have multiple writers which want to write different pieces of information to amundsen
that are either very company specific and thus would never be directly integrated into amundsen or require long form text
to properly convey the information.

What are some more specific examples of what could be used for this?
- You have an existing process that generates quality reports for a dataset that you want to embed in the table page.
- You have a process that detects pii information (also adding the appropriate tag/badge) but also generates a simple
report to provide context.
- You have extended table information that is applicable to your datastore which you want to scrape and provide in the
table page

Programmatic Descriptions are referred to by a "description source" which is a unique identifier.
You can then configure the descriptions to have a custom order in the config.py file like so:
```
PROGRAMMATIC_DISPLAY = {
"s3_crawler": {
"display_order": 0
},
"quality_service": {
"display_order": 1
},
"doesnt_exist": {
"display_order": 2
}
}
```
description sources not mentioned in the configuration will be alphabetically placed at the end of the above list. If `PROGRAMMATIC_DISPLAY` is left at `None` all added fields are still showing up, so that display is entirely dynamically data-driven without configuration. Meaning configuration merely adds the (nice) benefit of setting display order.

Here is a screenshot of what it would look like in the bottom left here:
![programmatic_description](img/programmatic_descriptions.png)
Binary file added docs/img/programmatic_descriptions.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
42 changes: 38 additions & 4 deletions tests/unit/api/metadata/test_v0.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@ def setUp(self) -> None:
'schema': 'test_schema',
'name': 'test_table',
'description': 'This is a test',
'programmatic_descriptions': [],
'programmatic_descriptions': [
{'source': 'c_1', 'text': 'description c'},
{'source': 'a_1', 'text': 'description a'},
{'source': 'b_1', 'text': 'description b'}
],
'tags': [],
'table_readers': [
{'user': {'email': '[email protected]', 'first_name': None, 'last_name': None}, 'read_count': 100}
Expand All @@ -75,7 +79,7 @@ def setUp(self) -> None:
'name': 'test_name',
'id': 'test_id',
'description': 'This is a test'
},
}
}
self.expected_parsed_metadata = {
'badges': [],
Expand All @@ -85,7 +89,6 @@ def setUp(self) -> None:
'name': 'test_table',
'key': 'test_db://test_cluster.test_schema/test_table',
'description': 'This is a test',
'programmatic_descriptions': [],
'tags': [],
'table_readers': [
{
Expand Down Expand Up @@ -119,6 +122,20 @@ def setUp(self) -> None:
'is_editable': True
}
],
"programmatic_descriptions": [
{
'source': 'a',
'text': 'description a'
},
{
'source': 'b',
'text': 'description b'
},
{
'source': 'c',
'text': 'description c'
},
],
'table_writer': {
'application_url': 'https://test-test.test.test',
'name': 'test_name',
Expand All @@ -131,7 +148,24 @@ def setUp(self) -> None:
],
'source': '/source',
'is_editable': True,
'last_updated_timestamp': None,
'last_updated_timestamp': None
}

self.expected_programmatic_descriptions_with_config = {
"programmatic_descriptions": [
{
'source': 'a',
'text': 'description a'
},
{
'source': 'b',
'text': 'description b'
},
{
'source': 'c',
'text': 'description c'
},
]
}
self.mock_tags = {
'tag_usages': [
Expand Down
Loading

0 comments on commit ef49b88

Please sign in to comment.