Ele 1804 source freshness result description calculate#1188
Conversation
ELE-1804 Calculate result description for source freshness run
Calculate source freshness result description to the alerts and to the report data.
Text:
In case of an error, show the error |
|
👋 @NoyaArie |
IDoneShaveIt
left a comment
IDoneShaveIt
left a comment
There was a problem hiding this comment.
Looking good overall.
Few small comments + lets add some tests to the time utils you touched (this is an important part of the system and we need to start make sure those methods are stable!)
| return isoformat_datetime | ||
|
|
||
|
|
||
| def convert_datetime_utc_str_to_timezone_datatime( |
There was a problem hiding this comment.
We already have the following 2 methods:
convert_utc_iso_format_to_datetime
convert_utc_time_to_timezone
You can use both or at least call those 2 inside this method instead of rewriting the logic that might be change in one place but be left the same on the other 🙂
There was a problem hiding this comment.
If we do keep this method that calls the other methods, can you add tests for it please 🙂
| return partial_iso_format_time | ||
|
|
||
|
|
||
| def get_formatted_timedelta(time_ago_in_s: float) -> str: |
There was a problem hiding this comment.
Just for later use - does it work with int as well?
If it does I think we should add it in the typing as mypy can start scream on other users that will use it due to it 🤔
There was a problem hiding this comment.
Can we add tests for that?
| snapshotted_at="2022-10-11 10:00:00", | ||
| max_loaded_at="2022-10-11 10:00:00", | ||
| max_loaded_at_time_ago_in_s="123123", | ||
| max_loaded_at_time_ago_in_s=123123, |
There was a problem hiding this comment.
I see that we add those fields but do we test that we get the wanted result from them?
| else None | ||
| ) | ||
| self.max_loaded_at_str = ( | ||
| self.max_loaded_at.strftime(DATETIME_FORMAT) if self.max_loaded_at else None |
There was a problem hiding this comment.
Lets extend the util convert_datetime_utc_str_to_timezone_str to support with_time_zone which by default is false to maintain backwards, instead of using the strftime all the time.
If with_time_zone is false - use DATETIME_FORMAT
else - use DATETIME_WITH_TIMEZONE_FORMAT
NoyaArie
force-pushed
the
ele-1804-source-freshness-result-description-calculate
branch
from
756a099 to
3f75d5b
Compare
NoyaArie
force-pushed
the
ele-1804-source-freshness-result-description-calculate
branch
from
3f75d5b to
1ccc1f8
Compare
NoyaArie
force-pushed
the
ele-1804-source-freshness-result-description-calculate
branch
from
1ccc1f8 to
3a9ff60
Compare
NoyaArie
deleted the
ele-1804-source-freshness-result-description-calculate
branch
* pluralize_string * add time functions * result description in source freshness alert * tests * use generated at * update text * fix text * change text * refactor * add tests
* Ele 1707 add source freshness as test part 1 (#1202) * get invocation from filter in invocations api * get test invocation from api and use in report * get_test_results get invocation id * cleanup * extract test results totals * extract test runs totals * refactor * Ele 1804 source freshness result description calculate (#1188) * pluralize_string * add time functions * result description in source freshness alert * tests * use generated at * update text * fix text * change text * refactor * add tests * tables_seasonality: small bugfix * add last generated at and complied code (#1212) * Ele-1865 add materialization and patch_path to the report models (#1213) * add fields * update * Fix typo * Updated dbt package revision. * Updated dbt package revision. * Move `enriched_exposures` and `elementary_exposures` from dbt package to `edr` cli * dbt_runner: remove quiet=True from _run_deps_if_needed, we need the logs * dbt_runner: add capture_output=True by default for deps * don't use dbt ls for statuses and resource types filter * fix getting owner alerts from normalized alert directly * add more selector types * Depends on macro: use enriched_exposures if needed * detection delay # Conflicts: # docs/guides/anomaly-detection-tests/volume-anomalies.mdx # docs/mint.json * change image for detection delay * Update add-exposure-tests.mdx * added * update latest version * New intro and quickstart (#1265) * WIP * WIP * fix * pretty * pretty * pretty * pretty * Update mint.json * replace URLs (#1272) * Cloud slack * Cloud slack * Docs refactor (#1291) * first draft - add data tests and features sections * add more features (TBD) * test configuration * test configuration docs * first draft - add data tests and features sections * add more features (TBD) * test configuration * test configuration docs * tests * changes * changes * oss intro * oss intro * buttons in intros * Cloud features * Cloud features * Cloud features * fixed broken links * prettier --------- Co-authored-by: Ella Katz <ella@elementary-data.com> * prettier * supported adapters * cloud tags * fix broken link * fix * pre commit fix * document columns and disabling columns autoupload --------- Co-authored-by: Noy Arie <noyarie1992@gmail.com> Co-authored-by: Itamar Hartstein <haritamar@gmail.com> Co-authored-by: Peter Lukacs <lukacs.peter.andras@gmail.com> Co-authored-by: Elon Gliksberg <elongliks@gmail.com> Co-authored-by: erikzaadi <erik@elementary-data.com> Co-authored-by: belle-crisp <belledeveer@crisp.nl> Co-authored-by: Daniel Pollak <daniel@elementary-data.com> Co-authored-by: Maayan Salom <maayansalom@gmail.com>
2 participants
No description provided.