ELE 518 Don't add details of Passed tests in Warnings by RoiTabach · Pull Request #751 · elementary-data/elementary

RoiTabach · 2023-03-15T12:44:04Z

No description provided.

This will help when calling those classes from Airflow - we don't pass tracking there and don't have access to MockTracking from elementary/tests

linear · 2023-03-15T12:44:08Z

ELE-518 Passed tests are listed with the Warnings in edr monitor report slack summary

Describe the bug
In the slack message that is sent with the report, we count correctly which of the tests are passed/failed/warning. However, when creating the details section, we count passed tests along with the warnings.

To Reproduce
Steps to reproduce the behavior:

Run dbt and have a small number of Passing tests. (I added a not_null test on the one model from our integration tests and ran dbt build -s one
Run edr monitor send-report
Look at the summary numbers
See the passed tests are counted properly but listed with the Warnings.

Expected behavior
No details for passed tests.

Screenshots

Environment (please complete the following information):

edr Version: [e.g. 0.5.3], can be found by running pip show elementary-data
dbt package Version: [e.g. 0.4.1], can be found in packages.yml file

Additional context
Raised in Slack thread https://elementary-community.slack.com/archives/C02CTC89LAX/p1678878540310359
I believe the fact we truncate the report on Slack if it's got too many items caused us to not see it even tho it was there for a long time - since we usually test with 100+ issues so the details view is usually not seen.

github-actions · 2023-03-15T12:44:18Z

👋 @RoiTabach
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

Fixing incorrect group_by flag documentation for slack

Add target_path flag option for edr files and logs

…bled Not initializing the tracking object if it's disabled.

…-sending-html-report-if Fixed bug where bucket URL was none and report wasn't sent as an html

…ow-task-to-send-alerts Allow `tracking` to be Optional

…ile settings

…ebug-entrypoint-to-the Added edr monitor debug to help our users debug their elementary prof…

IDoneShaveIt

Approved with one small comment 🙂

IDoneShaveIt · 2023-03-22T07:44:43Z

elementary/monitor/data_monitoring/report/slack_report_summary_message_builder.py

                failed_tests_details.extend(
                    self._get_test_result_details_block(test, include_description)
                )
+            elif test.status == "pass":


I think it will be better to replace the elif pass with elif warning and the rest will be skipped :)

…with-the' into ele-518-passed-tests-are-listed-with-the

RoiTabach · 2023-03-22T08:35:44Z

opened #767 instead of this one

RoiTabach and others added 5 commits March 9, 2023 15:06

make DataMonitoring OK with getting None tracking

c738505

This will help when calling those classes from Airflow - we don't pass tracking there and don't have access to MockTracking from elementary/tests

make tracking Optional

0ef8327

Extract _init_dbt_project to a member function in DataMonitoring

17707ec

Fixed bug where bucket URL was none and report wasn't sent as an html

351bed7

ELE 518 Don't add details of Passed tests in Warnings

a89eeee

Maayan-s linked an issue Mar 15, 2023 that may be closed by this pull request

[ELE-518] Passed tests are listed with the Warnings in edr monitor report slack summary #750

Closed

vishaalkk and others added 22 commits March 15, 2023 10:56

Fixing incorrect group_by flag documentation for slack

117497e

Merge pull request #753 from vishaalkk/Fix-slack-alert-documentation

4042c96

Fixing incorrect group_by flag documentation for slack

Added @vishaalkk to contributors! 🎉

99f742a

Add target_path flag option for edr files and logs

75bc7e4

Merge pull request #747 from elementary-data/ele-303-add-target-path

ba09d5e

Add target_path flag option for edr files and logs

Added @noaKurman and @sadahry to contributors 🎉

df2661b

Not initializing the tracking object if it's disabled.

8d1cfc0

Merge pull request #757 from elementary-data/no-init-tracking-if-disa…

6e98bd4

…bled Not initializing the tracking object if it's disabled.

Add files via upload

55946b1

Update README.md

c0ae5a3

Delete report_ui.gif

10b715c

Add files via upload

42e7824

Update README.md

1e6c20e

Fixing incorrect group_by flag documentation for slack

65cfd5a

Added @vishaalkk to contributors! 🎉

c09ae06

Add target_path flag option for edr files and logs

f347b88

Added @noaKurman and @sadahry to contributors 🎉

8604311

Not initializing the tracking object if it's disabled.

6948194

force slack-sdk 3.20.1

650f3c6

pre-commit fix

f7cc303

add a unit test

dd009b1

fix failing test

8ff107a

RoiTabach and others added 11 commits March 16, 2023 18:49

Merge pull request #746 from elementary-data/ele-502-fix-bug-when-not…

0eaae19

…-sending-html-report-if Fixed bug where bucket URL was none and report wasn't sent as an html

Added tracking interface and anonymous cli tracking objects

874408d

pre-commit fixes

6328460

Added set_group_id to the tracking ifc

4e3a31b

Merge pull request #744 from elementary-data/ele-466-k8s-create-airfl…

b6201ac

…ow-task-to-send-alerts Allow `tracking` to be Optional

Added 'proc_err' as a property on 'DbtCommandError'.

77cdd53

Added 'test_conn' macro.

816fb3b

Update question-singular-tests-config.mdx

d3eafd7

Added edr monitor debug to help our users debug their elementary prof…

f572f62

…ile settings

Added exit code

18bc3db

Merge pull request #765 from elementary-data/ele-556-add-elementary-d…

a63d8ec

…ebug-entrypoint-to-the Added edr monitor debug to help our users debug their elementary prof…

IDoneShaveIt approved these changes Mar 22, 2023

View reviewed changes

vishaalkk and others added 11 commits March 22, 2023 10:21

Fixing incorrect group_by flag documentation for slack

9d6530b

Added @vishaalkk to contributors! 🎉

1e34551

Add target_path flag option for edr files and logs

db54755

Added @noaKurman and @sadahry to contributors 🎉

9b61ec2

Not initializing the tracking object if it's disabled.

12435fd

Idan CR

288c17c

ELE 518 Don't add details of Passed tests in Warnings

5c545e6

add a unit test

bedaa31

fix failing test

e981ac5

Idan CR

38413ca

Merge remote-tracking branch 'origin/ele-518-passed-tests-are-listed-…

7778cc1

…with-the' into ele-518-passed-tests-are-listed-with-the

RoiTabach closed this Mar 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ELE 518 Don't add details of Passed tests in Warnings#751

ELE 518 Don't add details of Passed tests in Warnings#751
RoiTabach wants to merge 49 commits intomasterfrom
ele-518-passed-tests-are-listed-with-the

RoiTabach commented Mar 15, 2023

Uh oh!

linear bot commented Mar 15, 2023

Uh oh!

github-actions bot commented Mar 15, 2023

Uh oh!

IDoneShaveIt left a comment

Uh oh!

IDoneShaveIt Mar 22, 2023

Uh oh!

RoiTabach commented Mar 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

RoiTabach commented Mar 15, 2023

Uh oh!

linear bot commented Mar 15, 2023

Uh oh!

github-actions bot commented Mar 15, 2023

Uh oh!

IDoneShaveIt left a comment

Choose a reason for hiding this comment

Uh oh!

IDoneShaveIt Mar 22, 2023

Choose a reason for hiding this comment

Uh oh!

RoiTabach commented Mar 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants