[MAINTENANCE] Instrument test_yaml_config() #2981

anthonyburdi · 2021-07-01T20:27:51Z

Changes proposed in this pull request:

Add usage statistics instrumentation to test_yaml_config() (golden path and diagnostic_info for errors)
Update anonymizers to handle new v3 API data types for use in the test_yaml_config() instrumentation
Anonymizers can now check if the parent class of a given class is recognized, this is used to determine if a Datasource is compatible with the V2 or V3 API.
Special handling for SimpleSqlalchemyDatasource is included
Supported types for test_yaml_config are now class variables of BaseDataContext
Tests for the above

Definition of Done

Please delete options that are not relevant.

My code follows the Great Expectations style guide
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added unit tests where applicable and made sure that new and existing tests are passing.
I have run any local integration tests and made sure that nothing is broken.

…t_yaml_config()

…l_config()

…s_messages

…e GE class

… supported

…s_messages

…bclass

netlify · 2021-07-01T20:27:57Z

✔️ Deploy Preview for knoxpod ready!

🔨 Explore the source changes: b2adada

🔍 Inspect the deploy log: https://app.netlify.com/sites/knoxpod/deploys/60e47b793483a50007bbb470

😎 Browse the preview: https://deploy-preview-2981--knoxpod.netlify.app

github-actions · 2021-07-01T20:28:21Z

HOWDY! This is your friendly 🤖 CHANGELOG bot 🤖

Please don't forget to add a clear and succinct description of your change under the Develop header in docs/changelog.rst, if applicable. This will help us with the release process. See the Contribution checklist in the Great Expectations documentation for the type of labels to use!

✨ Thank you! ✨

Shinnnyshinshin · 2021-07-01T20:56:07Z

great_expectations/core/usage_statistics/schemas.py

+            "additionalProperties": True,
+            # we don't want this to be true, but this is required to allow show_cta_footer
+            # Note AJB-20201218 show_cta_footer was removed in v 0.9.9 via PR #1249
+            "required": ["parent_class"],


Note : these lines were moved here from another section. Don't worry about this :)

Shinnnyshinshin · 2021-07-01T20:56:52Z

great_expectations/core/usage_statistics/schemas.py

+                "anonymized_execution_engine": {
+                    "$ref": "#/definitions/anonymized_class_info"
+                },
+                "anonymized_data_connectors": {
+                    "type": "array",
+                    "maxItems": 1000,
+                    "items": {"$ref": "#/definitions/anonymized_class_info"},
+                },


Note : this was added by this PR (excuse the strange diff)

Shinnnyshinshin · 2021-07-01T20:59:29Z

great_expectations/core/usage_statistics/schemas.py

+                    "__substitution_error__",
+                    "__yaml_parse_error__",
+                    "__custom_subclass_not_core_ge__",
+                    "__class_name_not_provided__",


more descriptive errors:

substitution : variable substitution error (I dont exist)

yaml parse : tabs are off in yaml

not core GE : the class is not in the list in anonymizer or datacontext

class_name not provided : we didn't have a class name for you

Shinnnyshinshin · 2021-07-01T21:39:57Z

great_expectations/data_context/data_context.py

+        except Exception as e:
+            usage_stats_event_payload: dict = {
+                "diagnostic_info": ["__substitution_error__"],
+            }
+            send_usage_message(
+                data_context=self,
+                event=usage_stats_event_name,
+                event_payload=usage_stats_event_payload,
+                success=False,
+            )
+            raise e


this is how we generate a substitution_error

Shinnnyshinshin · 2021-07-01T21:40:38Z

great_expectations/data_context/data_context.py

+        except Exception as e:
+            usage_stats_event_payload: dict = {
+                "diagnostic_info": ["__yaml_parse_error__"],
+            }
+            send_usage_message(
+                data_context=self,
+                event=usage_stats_event_name,
+                event_payload=usage_stats_event_payload,
+                success=False,
+            )


yaml_parrse_error generated here

Shinnnyshinshin · 2021-07-01T21:42:05Z

great_expectations/data_context/data_context.py

+                if class_name == "SimpleSqlalchemyDatasource":
+                    # Use the raw config here, defaults will be added in the anonymizer
+                    usage_stats_event_payload = (
+                        datasource_anonymizer.anonymize_simple_sqlalchemy_datasource(
+                            name=datasource_name, config=config
+                        )
+                    )


SimpleSqlalchemyDatasource has a different class inheritance so handled separately

Shinnnyshinshin · 2021-07-01T21:46:28Z

great_expectations/data_context/data_context.py

+                else:
+                    # If class_name is not a supported type or subclass of a supported type,
+                    # mark it as custom with no additional information since we can't anonymize
+                    usage_stats_event_payload[
+                        "diagnostic_info"
+                    ] = usage_stats_event_payload.get("diagnostic_info", []) + [
+                        "__custom_subclass_not_core_ge__"
+                    ]


not a core ge class and not a subclass .. then we dont know what it is. error.

Shinnnyshinshin · 2021-07-01T21:57:10Z

tests/data_context/test_data_context_test_yaml_config_usage_stats.py

+# Test usage stats for test_yaml_config
+# - [x] test_test_yaml_config_usage_stats_substitution_error
+# - [x] test_test_yaml_config_usage_stats_yaml_parse_error
+# See test_data_context_test_yaml_config.test_config_with_yaml_error()
+# - [x] test_test_yaml_config_usage_stats_store_type
+# See test_data_context_test_yaml_config.test_expectations_store_with_filesystem_store_backend()
+# - [NA] test_test_yaml_config_usage_stats_datasource_type_v2
+# - [x] test_test_yaml_config_usage_stats_datasource_type_v3


this is ~ sniff ~ beautiful :)

tests/integration/usage_statistics/test_usage_statistics_messages.py

Shinnnyshinshin

A true tour de force :) thank you for the well-thought-out design, implementation and testing. Leaving a few non-blocking requests for comments, first in the DataConnectorAnonymizer and also in the classes that are used for testing (which we talked about last week). Otherwise LGTM! :)

great_expectations/core/usage_statistics/anonymizers/data_connector_anonymizer.py

tests/core/usage_statistics/test_anonymizer.py

tests/data_context/fixtures/plugins/my_custom_expectations_store.py

tests/data_context/fixtures/plugins/my_custom_simple_sqlalchemy_datasource_class.py

tests/data_context/fixtures/plugins/my_custom_v2_api_datasource.py

tests/data_context/fixtures/plugins/my_custom_v3_api_datasource.py

…s_messages # Conflicts: # docs_rtd/changelog.rst

…s_messages

Instrument test_yaml_config() and update Anonymizers

anthonyburdi added 30 commits June 18, 2021 16:03

Add usage stats messages to test_yaml_config

079179a

Add "data_context.test_yaml_config" event to schema

c0b8fc2

Test example "data_context.test_yaml_config" events

341e1bb

Add ERRONEOUS_CONFIG and CUSTOM_CONFIG messages for test_yaml_config

2659616

Add ERRONEOUS_CONFIG and CUSTOM_CONFIG messages for test_yaml_config

6fcf9cf

Add to existing tests to ensure usage stats messages are sent for tes…

f697350

…t_yaml_config()

Add to existing tests to ensure usage stats messages are sent for tes…

a529e14

…t_yaml_config()

WIP add "diagnostic_info" and standardize supported types in test_yam…

a2f55f0

…l_config()

Merge branch 'develop' into MAINTENANCE/GEA-1/GEA-7/update_usage_stat…

eea80f4

…s_messages

Add diagnostic message types

1698d54

Merge branch 'develop' into MAINTENANCE/GEA-1/GEA-7/update_usage_stat…

e6caaa0

…s_messages

Use anonymizers in test_yaml_config()

c563cf3

Update schemas for new data_context.test_yaml_config event

1fce61d

WIP test updates

48f03da

Use Anonymizers to determine if a custom class is a subclass of a cor…

92ccbfa

…e GE class

Handle SimpleSqlalchemyDatasource in datasource_anonymizer

7ae669d

Handle anonymized_name

53dc535

Handle subclasses of core GE types

1d3089a

Handle SimpleSqlalchemyDatasource

81e8e47

Custom expectation_suite fixture plugin for tests

0a3b913

diagnostic_info is a list, add parent_class on error if class_name is…

de2bf15

… supported

Align event fixtures with anonymizer output

15dea73

WIP add test_yaml_config usage_stats tests not captured elsewhere

45ebf85

Rename to expectations_store, cleanup

ad4fdc5

test when no class name is provided for custom class

cad87bd

test when custom type is not a core ge class or subclass thereof

f29d85d

test for usage_stats when SimpleSqlalchemyDatasource is subclassed

a101209

Merge branch 'develop' into MAINTENANCE/GEA-1/GEA-7/update_usage_stat…

e5c1df0

…s_messages

Ignore custom v2 Datasources

1197b87

Custom v2 Datasources supported

3f8b8d9

anthonyburdi added 7 commits July 1, 2021 12:18

Linting fixtures

2bd6519

test DatasourceAnonymizer

699217b

Remove self_check from MyCustomV2ApiDatasource and related changes

d847701

Merge branch 'develop' into MAINTENANCE/GEA-1/GEA-7/update_usage_stat…

82261a6

…s_messages

changelog

9929203

add test_anonymizer__is_parent_class_recognized

41b7f09

fix test_test_yaml_config_usage_stats_simple_sqlalchemy_datasource_su…

f8e4e27

…bclass

anthonyburdi self-assigned this Jul 1, 2021

anthonyburdi requested a review from Shinnnyshinshin July 1, 2021 20:28

Shinnnyshinshin reviewed Jul 1, 2021

View reviewed changes

tests/integration/usage_statistics/test_usage_statistics_messages.py Show resolved Hide resolved

anthonyburdi marked this pull request as ready for review July 1, 2021 22:36

anthonyburdi enabled auto-merge (squash) July 1, 2021 22:38

Shinnnyshinshin approved these changes Jul 5, 2021

View reviewed changes

anthonyburdi added 4 commits July 6, 2021 11:07

Merge branch 'develop' into MAINTENANCE/GEA-1/GEA-7/update_usage_stat…

fd9cadf

…s_messages # Conflicts: # docs_rtd/changelog.rst

Add comments per Will's suggestions

522d2af

Linting fixtures

f5741ea

Merge branch 'develop' into MAINTENANCE/GEA-1/GEA-7/update_usage_stat…

b2adada

…s_messages

anthonyburdi merged commit d5fab68 into develop Jul 6, 2021

anthonyburdi deleted the MAINTENANCE/GEA-1/GEA-7/update_usage_stats_messages branch July 6, 2021 16:49

pasmavie pushed a commit to pasmavie/great_expectations that referenced this pull request Jul 13, 2021

[MAINTENANCE] Instrument test_yaml_config() (great-expectations#2981)

e274f29

Instrument test_yaml_config() and update Anonymizers

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MAINTENANCE] Instrument test_yaml_config() #2981

[MAINTENANCE] Instrument test_yaml_config() #2981

anthonyburdi commented Jul 1, 2021 •

edited

Loading

netlify bot commented Jul 1, 2021 •

edited

Loading

github-actions bot commented Jul 1, 2021

Shinnnyshinshin Jul 1, 2021

Shinnnyshinshin Jul 1, 2021

Shinnnyshinshin Jul 1, 2021

Shinnnyshinshin Jul 1, 2021

Shinnnyshinshin Jul 1, 2021

Shinnnyshinshin Jul 1, 2021

Shinnnyshinshin Jul 1, 2021

Shinnnyshinshin Jul 1, 2021

Shinnnyshinshin left a comment

[MAINTENANCE] Instrument test_yaml_config() #2981

[MAINTENANCE] Instrument test_yaml_config() #2981

Conversation

anthonyburdi commented Jul 1, 2021 • edited Loading

Definition of Done

netlify bot commented Jul 1, 2021 • edited Loading

github-actions bot commented Jul 1, 2021

HOWDY! This is your friendly 🤖 CHANGELOG bot 🤖

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shinnnyshinshin left a comment

Choose a reason for hiding this comment

anthonyburdi commented Jul 1, 2021 •

edited

Loading

netlify bot commented Jul 1, 2021 •

edited

Loading