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

Mode Dashboard execution timestamp and state #197

Merged
merged 13 commits into from
Feb 24, 2020

Conversation

jinhyukchang
Copy link
Contributor

@jinhyukchang jinhyukchang commented Feb 20, 2020

Summary of Changes

Add:

  • ModeDashboardExecutionsExtractor. An extractor that extracts the latest execution timestamp in epoch format and state of execution of Mode dashboard.
  • DashboardExecution. A model represents Execution of Dashboard
  • TimestampStringToEpoch. A transformer to convert string timestamp (e.g: ISO8601) to epoch
  • DictToModel. A transformer to convert Dictionary to Model.
  • Utilities and Constants to reduce duplicates.

Update:

  • RestApiQuery. Added support of OR ( | ) in JSONPATH for JSONPATH's result is in different order between AND (,) and OR (|). More detail with example in docstring.

Tests

Unit test, Integration test.

Documentation

docstring

CheckList

Make sure you have checked all steps below to ensure a timely review.

  • PR title addresses the issue accurately and concisely. Example: "Updates the version of Flask to v1.0.2"
  • PR includes a summary of changes.
  • PR adds unit tests, updates existing unit tests, OR documents why no test additions or modifications are needed.
  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
  • PR passes make test

@codecov-io
Copy link

codecov-io commented Feb 20, 2020

Codecov Report

Merging #197 into master will increase coverage by 7.75%.
The diff coverage is 44.82%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #197      +/-   ##
==========================================
+ Coverage   72.42%   80.18%   +7.75%     
==========================================
  Files          73       79       +6     
  Lines        3511     3669     +158     
  Branches      383      391       +8     
==========================================
+ Hits         2543     2942     +399     
+ Misses        880      616     -264     
- Partials       88      111      +23
Impacted Files Coverage Δ
...r/dashboard/mode_analytics/mode_dashboard_utils.py 0% <0%> (ø)
...e_analytics/mode_dashboard_executions_extractor.py 0% <0%> (ø)
...shboard/mode_analytics/mode_dashboard_constants.py 0% <0%> (ø)
...shboard/mode_analytics/mode_dashboard_extractor.py 0% <0%> (ø)
...atabuilder/models/dashboard/dashboard_execution.py 71.05% <71.05%> (ø)
databuilder/rest_api/rest_api_query.py 82.85% <77.77%> (-2.33%) ⬇️
...tabuilder/transformer/timestamp_string_to_epoch.py 87.5% <87.5%> (ø)
databuilder/transformer/dict_to_model.py 94.11% <94.11%> (ø)
databuilder/task/neo4j_staleness_removal_task.py 56.98% <0%> (ø) ⬆️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01a0f96...9efb82c. Read the comment docs.

@jinhyukchang jinhyukchang changed the title [WIP] Mode Dashboard execution timestamp and state Mode Dashboard execution timestamp and state Feb 21, 2020
Copy link
Member

@feng-tao feng-tao left a comment

Choose a reason for hiding this comment

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

put some init comments...

databuilder/rest_api/rest_api_query.py Outdated Show resolved Hide resolved
tests/unit/transformer/test_dict_to_model_transformer.py Outdated Show resolved Hide resolved
# Reports
url = 'https://app.mode.com/api/{organization}/spaces/{dashboard_group_id}/reports'
json_path = '(_embedded.reports[*].token) | (_embedded.reports[*]._links.last_run.href)'
field_names = ['dashboard_id', 'last_run_resource_path']
Copy link
Member

Choose a reason for hiding this comment

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

does it mean it used dashboard_id as a variable to reference (_embedded.reports[].token) while last_run_resource_path on (_embedded.reports[]._links.last_run.href) ?

Copy link
Contributor Author

@jinhyukchang jinhyukchang Feb 21, 2020

Choose a reason for hiding this comment

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

field_names refer to the dictionary keys here.

Where in this case, the value extracted from _embedded.reports[*].token will be placed to dashboard_id and the value extracted from _embedded.reports[*]._links.last_run.href will be placed to last_run_resource_path

e.g: {'dashboard_id': 'foo', 'last_run_resource_path': '/foo/bar/baz'}

Hope this helps.

databuilder/extractor/dashboard/mode_dashboard_utils.py Outdated Show resolved Hide resolved
databuilder/rest_api/rest_api_query.py Outdated Show resolved Hide resolved
LOGGER.info('Calling URL {}'.format(url))
response = requests.get(url, **self._params)
response.raise_for_status()
return response

@classmethod
def _compute_sub_records(self,
Copy link
Member

Choose a reason for hiding this comment

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

this function is a bit cryptic to me...I am wondering any other way to write it cleaner...

Copy link
Member

Choose a reason for hiding this comment

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

for and case, here is an example per offline sync:

In [39]: a
Out[39]: [1, 2, 3, 4, 5, 6]

In [40]:  b=[a[i:i+3] for i in range(0, len(a), 3)]

In [41]: b
Out[41]: [[1, 2, 3], [4, 5, 6]]

Copy link
Member

Choose a reason for hiding this comment

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

3 is the numbers of the field name here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, Tao!
Simplified the method and let me know 😄

@jinhyukchang
Copy link
Contributor Author

@feng-tao PR is updated and let me know.

@jinhyukchang jinhyukchang merged commit 6c4bd0b into master Feb 24, 2020
@jinhyukchang jinhyukchang deleted the jinchang-mode_dashboard_executions branch February 24, 2020 21:26
PabTorre pushed a commit to PabTorre/amundsendatabuilder that referenced this pull request May 29, 2020
ccarterlandis pushed a commit to Gusto/amundsendatabuilder_contrib that referenced this pull request Aug 7, 2020
* Patch to fix backward incompatible issue on table metadata (amundsen-io#195)

* Backward compatible patch

* Increment version

* Update

* escaping backslashes in neo4j publisher (amundsen-io#191)

* escaping backslashes in neo4j publisher

* removing an extra backslash

* Bumping versions to 1.5.3

* Mode Dashboard extractor with Generic REST API Query (amundsen-io#194)

* Initial check in on REST API Query

* Working version

* docstring

* Update

* Update

* Make unit test happy

* Update docstring

* Update

* Update

* Update

* Adding unit tests

* Updated README.md

* jsonpath_rw to extra_requires

* update models for 2.0.0 (amundsen-io#188)

* update models for 2.0.0

schema_name -> schema in table model
update search model to use schema, last_updated_timestamp

* update name -> full_name

* Mode Dashboard execution timestamp and state (amundsen-io#197)

* Initial commit

* Update

* Update

* Update

* flake8

* Update docstring

* Update

* Update

* Add mode_dashboard_constants and mode_dashboard_utils

* Add more to Util

* Added back model class to ModeDashboardExtractor

* flake8

* Address PR comments

* Fix Sample Scripts and Data (amundsen-io#199)

* Change schema_name -> schema in sample_table_programmatic_source.csv

* Update schema_name -> schema in sample_data_loader

* Change name -> full_name in  sample_user.csv

* Change name -> full_name in sample_data_loader.py

* Retrigger CLA Check

* Fix broken HiveTableMetadataExtractor. (amundsen-io#201)

* Fix.

* Update corresponding tests.

* Replace schema_name with  and fix presto as well.

* Replace unit test with schema, since we renamed schema_name back to .

* Update setup.py

Co-authored-by: Tao Feng <[email protected]>

* Follow up patches on v2 field name changes (amundsen-io#202)

* Mode dashboard owner (amundsen-io#200)

* Mode dashboard owner

* Not having dashboard_owner to create User node

* Update

* Support unicode in file_system_neo4j_csv_loader (amundsen-io#203)

* Support unicode in file_system_neo4j_csv_loader

* increment version

* Update

* Update

* Let Python3 use csv not unicodecsv

* Add badges to Neo4jExtractor and elastic search  (amundsen-io#204)

* Add badges to Neo4jSearchExtractor

* update publisher to have badges

* update elastic search document

* fix typo

* update name

* filter tags by type

* typo

* do not filter tags because then i can't get badges on staging :|

* update tests

* fix tests

* use amunsen_common for elastic search index

* revert commit using amundsencommon

* add comment

* make backwards compatible

* remove badges from tags

* Mode Dashboard last successful execution (amundsen-io#205)

* Mode Dashboard last successful execution

* Increment version

* Remove py27 Antlar presto sql usage extractor (amundsen-io#208)

* Remove py27 Antlar presto sql usage extractor

* remove more

* remove pytest macro

* Removing Sample Csv Data Loader and Cleaning up Sample Data Loader (amundsen-io#206)

* refactored TableColumnCsvExtractor to the csv_extractors file.
removing sample_csv_data_loader.py
simplified sample_data_loader.py
Finally, modified sample data so that popular table shows by default again.

* Update sample_data_loader.py

Co-authored-by: Tao Feng <[email protected]>

* Added created timestamp and last modified timestamp on Dashboard (amundsen-io#207)

* Added created timestamp and last modified timestamp on Dashboard

* Remove owner and reload time from dashboard_metadata

* Move dashboard_metadata under dashboard package

* Update

* Update

* Added dashboard group url and dashboard url

* Added cluster node in Dashboard

* Optimize Neo4j Cypher query on Neo4jSearchDataExtractor (amundsen-io#213)

* Optimize Neo4j Cypher query on Neo4jSearchDataExtractor

* flake8

* Add Dashboard Table model (amundsen-io#210)

* Add Dashboard Table model

* Update dashboard_table.py

* Update test_dashboard_table.py

* bump version

* Remove antlr4 deps (amundsen-io#214)

* Update dashboard ES doc (amundsen-io#218)

* Remove neo4j dashboard/metric extractor (amundsen-io#219)

* Add default cypher query for user/dashboard entity to search extractor (amundsen-io#220)

* Update the query publish tag to match entity (amundsen-io#221)

* issue-320 documentation of models (amundsen-io#217)

* Start if issue-320 documentatino of models

* adding more documentation

* Adding BigQuery to the readme (amundsen-io#222)

* Adding Bigquery to the readme

* fixing link

* Rename sample_dag.py to hive_sample_dag.py (amundsen-io#223)

To clarify what this sample is about. 
A year ago this folder only had one file :)

* Adding Mode dashboard usage extractor and generic loader (amundsen-io#225)

* Adding Mode dashboard usage extractor and generic loader

* Update

* Increment version

* Add query and chart on Dashboard (amundsen-io#224)

* Add query and chart on Dashboard

* Update

* Increment version

* Update test case

* Add title field in user model (amundsen-io#227)

* Add title field in user model

* fix one more test

* Slight change from title to role name (amundsen-io#228)

* Update python version for databuilder (amundsen-io#229)

* Dashboard usage model (amundsen-io#226)

* Added Dashboard usage model

* Update

* Update

* Update

* Clarify ES publisher warning on first run (amundsen-io#230)

* Add role_name to user ES doc (amundsen-io#232)

* minor mods to get tests to pass

* tweak schema field after testing demo execution

* move Table into simple_sql_parser now that we're not trying to conform to a missing presto parser

* add role_name to sample_data_loader

Co-authored-by: Jin Hyuk Chang <[email protected]>
Co-authored-by: Luke Lowery <[email protected]>
Co-authored-by: friendtocephalopods <[email protected]>
Co-authored-by: Jacob Kim <[email protected]>
Co-authored-by: Robert Yi <[email protected]>
Co-authored-by: Tao Feng <[email protected]>
Co-authored-by: christina stead <[email protected]>
Co-authored-by: Tao Feng <[email protected]>
Co-authored-by: samshuster <[email protected]>
Co-authored-by: jornh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants