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

Add auxiliary etl query table #993

Merged
merged 5 commits into from
Jun 27, 2019
Merged

Conversation

maxalbert
Copy link
Contributor

Closes #988

I have:

  • Formatted any Python files with black
  • Brought the branch up to date with master
  • Added any relevant Github labels
  • Added tests for any new additions
  • Added or updated any relevant documentation
  • Added an Architectural Decision Record (ADR), if appropriate
  • Added an MPLv2 License Header if appropriate
  • Updated the Changelog

Description

This PR adds a table etl.post_etl_queries to record the outcome of queries run as part of the regular ETL process. I'm happy to tweak the name and structure of the table if we think something else is more appropriate, suggestions are welcome.

For the time being I have kept the data types for all columns (except cdr_date) as TEXT. Going forward one could imagine using enums for cdr_type and in particular for type_of_query_or_check (so that we enforce consistency in this column, which would help with querying the table). In the future we may also be more specific with the data type of the outcome column (e.g. use int/float/boolean/text depending on the actual query it records) but until the details become clearer and we split this auxiliary table into separate ones (probably as part of #640) this seemed to be the most generically useful approach.

Maximilian Albert added 3 commits June 27, 2019 17:25
@maxalbert maxalbert added enhancement New feature or request FlowDB Issues related to FlowDB labels Jun 27, 2019
@@ -78,6 +78,18 @@ CREATE TABLE etl.etl_records (
PRIMARY KEY (id)
);

CREATE TABLE etl.post_etl_queries (
cdr_date DATE,
Copy link
Member

Choose a reason for hiding this comment

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

Can we record the date of the check as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. 👍

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 made it a TIMESTAMPTZ rather than a DATE column because there is no harm in having the more precise info about when the check was run.

@maxalbert maxalbert added the ready-to-merge Label indicating a PR is OK to automerge label Jun 27, 2019
@codecov
Copy link

codecov bot commented Jun 27, 2019

Codecov Report

Merging #993 into master will decrease coverage by 6.63%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #993      +/-   ##
==========================================
- Coverage     100%   93.36%   -6.64%     
==========================================
  Files           1      137     +136     
  Lines          92     6892    +6800     
  Branches        7      699     +692     
==========================================
+ Hits           92     6435    +6343     
- Misses          0      332     +332     
- Partials        0      125     +125
Flag Coverage Δ
#flowapi_unit_tests 77.27% <ø> (?)
#flowauth_unit_tests 94.86% <ø> (?)
#flowclient_unit_tests 82.29% <ø> (?)
#flowetl_unit_tests 100% <ø> (?)
#flowkit_jwt_generator_unit_tests 100% <ø> (ø) ⬆️
#flowmachine_unit_tests 91.06% <ø> (?)
#integration_tests 60.16% <ø> (?)
Impacted Files Coverage Δ
...e/server/query_schemas/joined_spatial_aggregate.py 100% <0%> (ø)
...achine/core/server/query_schemas/daily_location.py 100% <0%> (ø)
...e/flowmachine/features/raster/raster_statistics.py 87.87% <0%> (ø)
...lowmachine/flowmachine/features/spatial/circles.py 100% <0%> (ø)
flowmachine/flowmachine/core/custom_query.py 100% <0%> (ø)
flowmachine/flowmachine/core/server/server.py 87.34% <0%> (ø)
...hine/features/location/unique_subscriber_counts.py 100% <0%> (ø)
...ne/core/server/query_schemas/radius_of_gyration.py 100% <0%> (ø)
...owmachine/features/subscriber/subscriber_degree.py 100% <0%> (ø)
flowapi/flowapi/main.py 96.1% <0%> (ø)
... and 126 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 4e247fc...87b6ed3. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jun 27, 2019

Codecov Report

Merging #993 into master will decrease coverage by 6.63%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #993      +/-   ##
==========================================
- Coverage     100%   93.36%   -6.64%     
==========================================
  Files           1      137     +136     
  Lines          92     6892    +6800     
  Branches        7      699     +692     
==========================================
+ Hits           92     6435    +6343     
- Misses          0      332     +332     
- Partials        0      125     +125
Flag Coverage Δ
#flowapi_unit_tests 77.27% <ø> (?)
#flowauth_unit_tests 94.86% <ø> (?)
#flowclient_unit_tests 82.29% <ø> (?)
#flowetl_unit_tests 100% <ø> (?)
#flowkit_jwt_generator_unit_tests 100% <ø> (ø) ⬆️
#flowmachine_unit_tests 91.06% <ø> (?)
#integration_tests 60.16% <ø> (?)
Impacted Files Coverage Δ
...e/server/query_schemas/joined_spatial_aggregate.py 100% <0%> (ø)
...achine/core/server/query_schemas/daily_location.py 100% <0%> (ø)
...e/flowmachine/features/raster/raster_statistics.py 87.87% <0%> (ø)
...lowmachine/flowmachine/features/spatial/circles.py 100% <0%> (ø)
flowmachine/flowmachine/core/custom_query.py 100% <0%> (ø)
flowmachine/flowmachine/core/server/server.py 87.34% <0%> (ø)
...hine/features/location/unique_subscriber_counts.py 100% <0%> (ø)
...ne/core/server/query_schemas/radius_of_gyration.py 100% <0%> (ø)
...owmachine/features/subscriber/subscriber_degree.py 100% <0%> (ø)
flowapi/flowapi/main.py 96.1% <0%> (ø)
... and 126 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 4e247fc...87b6ed3. Read the comment docs.

@maxalbert maxalbert merged commit 9b6efcf into master Jun 27, 2019
@maxalbert maxalbert deleted the add-auxiliary-etl-query-table branch June 27, 2019 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request FlowDB Issues related to FlowDB ready-to-merge Label indicating a PR is OK to automerge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add auxiliary table to record results of regular queries (e.g. run as part of daily ingestion)
2 participants