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.

Sign up for GitHub

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

Improve table & columns description formatting (#98) #79

Merged
merged 4 commits into from
Sep 25, 2019
Merged

Improve table & columns description formatting (#98) #79

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4e869b7
Improve table & columns description formatting (#98)
Mikhail-Ivanov Sep 20, 2019
28dc087
Improve table & columns description formatting (#98)
Mikhail-Ivanov Sep 20, 2019
a6d0c81
Improve table & columns description formatting (#98)
Mikhail-Ivanov Sep 20, 2019
fb02351
Merge branch 'master' into feature/table_description_format
feng-tao Sep 25, 2019
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
6 changes: 2 additions & 4 deletions metadata_service/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,14 @@ def create_app(*, config_module_class: str) -> Flask:
api.add_resource(PopularTablesAPI, '/popular_tables/')
api.add_resource(TableDetailAPI, '/table/<path:table_uri>')
api.add_resource(TableDescriptionAPI,
'/table/<path:table_uri>/description',
'/table/<path:table_uri>/description/<path:description_val>')
'/table/<path:table_uri>/description')
api.add_resource(TableTagAPI,
'/table/<path:table_uri>/tag',
'/table/<path:table_uri>/tag/<tag>')
api.add_resource(TableOwnerAPI,
'/table/<path:table_uri>/owner/<owner>')
api.add_resource(ColumnDescriptionAPI,
'/table/<path:table_uri>/column/<column_name>/description',
'/table/<path:table_uri>/column/<column_name>/description/<path:description_val>')
'/table/<path:table_uri>/column/<column_name>/description')
api.add_resource(Neo4jDetailAPI,
'/latest_updated_ts')
api.add_resource(TagAPI,
Expand Down
20 changes: 10 additions & 10 deletions metadata_service/api/column.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import json
from http import HTTPStatus
from typing import Iterable, Union

from flask_restful import Resource, reqparse
from flask import request
from flask_restful import Resource

from metadata_service.exception import NotFoundException
from metadata_service.proxy import get_proxy_client
Expand All @@ -13,24 +15,22 @@ class ColumnDescriptionAPI(Resource):
"""
def __init__(self) -> None:
self.client = get_proxy_client()

self.parser = reqparse.RequestParser()
self.parser.add_argument('description', type=str, location='json')

super(ColumnDescriptionAPI, self).__init__()

def put(self,
table_uri: str,
column_name: str,
description_val: str) -> Iterable[Union[dict, tuple, int, None]]:
column_name: str) -> Iterable[Union[dict, tuple, int, None]]:
"""
Updates column description
Updates column description (passed as a request body)
:param table_uri:
:param column_name:
:return:
"""
try:
description = json.loads(request.json).get('description')
Copy link
Contributor

@ttannis ttannis Sep 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

What do you all think about having a check for what happens if the description is None? Does self.client.put_column_description end up handling this (in that case no need for a check here), or will the description be deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've checked it locally (via Postman):

  1. {"description": ""} - this request payload will truncate description
  2. request payload with no "description" tag ends with {"msg": "Encountered exception: None"} and description data stay untouched

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for confirming the behavior for empty string and None. They behave as we expect, so no changes would be needed.

self.client.put_column_description(table_uri=table_uri,
column_name=column_name,
description=description_val)

description=description)
return None, HTTPStatus.OK

except NotFoundException:
Expand Down
14 changes: 6 additions & 8 deletions metadata_service/api/table.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import json
from http import HTTPStatus
from typing import Iterable, Mapping, Union, Any

from flask import request
from flask_restful import Resource, fields, reqparse, marshal

from metadata_service.exception import NotFoundException
Expand Down Expand Up @@ -135,10 +137,6 @@ class TableDescriptionAPI(Resource):
"""
def __init__(self) -> None:
self.client = get_proxy_client()

self.parser = reqparse.RequestParser()
self.parser.add_argument('description', type=str, location='json')

super(TableDescriptionAPI, self).__init__()

def get(self, table_uri: str) -> Iterable[Any]:
Expand All @@ -155,15 +153,15 @@ def get(self, table_uri: str) -> Iterable[Any]:
except Exception:
return {'message': 'Internal server error!'}, HTTPStatus.INTERNAL_SERVER_ERROR

def put(self, table_uri: str, description_val: str) -> Iterable[Any]:
def put(self, table_uri: str) -> Iterable[Any]:
"""
Updates table description
Updates table description (passed as a request body)
:param table_uri:
:param description_val:
:return:
"""
try:
self.client.put_table_description(table_uri=table_uri, description=description_val)
description = json.loads(request.json).get('description')
self.client.put_table_description(table_uri=table_uri, description=description)
return None, HTTPStatus.OK

except NotFoundException:
Expand Down
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from setuptools import setup, find_packages

__version__ = '1.0.19'
__version__ = '1.0.20'

requirements_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'requirements.txt')
with open(requirements_path) as requirements_file:
Expand Down
23 changes: 11 additions & 12 deletions tests/unit/api/test_redshit_disable_comment_edit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,26 @@
import json
import unittest
from http import HTTPStatus

from mock import patch
from metadata_service.api.table import TableDescriptionAPI
from metadata_service.api.column import ColumnDescriptionAPI
from tests.unit.test_basics import BasicTestCase


class RedshiftCommentEditDisableTest(BasicTestCase):

class RedshiftCommentEditDisableTest(unittest.TestCase):
def test_table_comment_edit(self) -> None:
with patch('metadata_service.api.table.get_proxy_client'):
tbl_dscrpt_api = TableDescriptionAPI()

table_uri = 'hive://gold.test_schema/test_table'
response = tbl_dscrpt_api.put(table_uri=table_uri, description_val='test')
self.assertEqual(list(response)[1], HTTPStatus.OK)
url = '/table/' + table_uri + '/description'
response = self.app.test_client().put(url, json=json.dumps({'description': 'test table'}))
self.assertEqual(response.status_code, HTTPStatus.OK)

def test_column_comment_edit(self) -> None:
with patch('metadata_service.api.column.get_proxy_client'):
col_dscrpt_api = ColumnDescriptionAPI()

table_uri = 'hive://gold.test_schema/test_table'
response = col_dscrpt_api.put(table_uri=table_uri, column_name='foo', description_val='test')
self.assertEqual(list(response)[1], HTTPStatus.OK)
column_name = 'foo'
url = '/table/' + table_uri + '/column/' + column_name + '/description'
response = self.app.test_client().put(url, json=json.dumps({'description': 'test column'}))
self.assertEqual(response.status_code, HTTPStatus.OK)


if __name__ == '__main__':
Expand Down