[MAINTENANCE] Misc cleanup of GX Cloud helpers #6352

cdkini · 2022-11-14T19:59:07Z

Changes proposed in this pull request:

CONTRACT and SUITE_VALIDATION_RESULT are legacy and have been slated for removal for some time.
Classes should utilize the GX prefix
get_context() tests should work when you have env vars set
- Let's delete them using monkeypatch so this is the case

Definition of Done

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 run any local integration tests and made sure that nothing is broken.

netlify · 2022-11-14T19:59:17Z

✅ Deploy Preview for niobium-lead-7998 ready!

Name	Link
🔨 Latest commit	`81dbce8`
🔍 Latest deploy log	https://app.netlify.com/sites/niobium-lead-7998/deploys/6373ad00e3e41f00084efd03
😎 Deploy Preview	https://deploy-preview-6352--niobium-lead-7998.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

ghost · 2022-11-14T20:00:24Z

👇 Click on the image for a new way to code review

Make big changes easier — review code in small groups of related files
Know where to start — see the whole change at a glance
Take a code tour — explore the change with an interactive tour
Make comments and review — all fully sync’ed with github

Try it now!

Legend

…_expectations into m/_/clean_up_cloud_rest_resource_enum

cdkini · 2022-11-14T20:58:30Z

tests/data_context/test_get_data_context.py

+    # Delete local env vars (if present)
+    for env_var in GXCloudEnvironmentVariable:
+        monkeypatch.delenv(env_var, raising=False)
+


Test fails if you have env vars set - let's ensure we clear out our environment for consistent behavior

This seems like something we might want to re-use in a fixture.
clean_cloud_env_vars / no_cloud_env 🤷

yup this was done shortly after!

cdkini · 2022-11-14T20:58:46Z

great_expectations/data_context/cloud_constants.py

+class GXCloudEnvironmentVariable(str, Enum):
    BASE_URL = "GE_CLOUD_BASE_URL"
    ORGANIZATION_ID = "GE_CLOUD_ORGANIZATION_ID"
    ACCESS_TOKEN = "GE_CLOUD_ACCESS_TOKEN"


-class GeCloudRESTResource(str, Enum):
+class GXCloudRESTResource(str, Enum):


Most of the changes here are from these renames to GX

cdkini · 2022-11-14T20:59:09Z

great_expectations/data_context/store/ge_cloud_store_backend.py

-        # Chetan - 20220811 - CONTRACT is deprecated by GX Cloud and is to be removed upon migration of E2E tests
-        GeCloudRESTResource.CONTRACT: "checkpoint_config",
-        GeCloudRESTResource.DATASOURCE: "datasource_config",
-        GeCloudRESTResource.DATA_CONTEXT: "data_context_config",
-        GeCloudRESTResource.DATA_CONTEXT_VARIABLES: "data_context_variables",
-        GeCloudRESTResource.EXPECTATION_SUITE: "suite",
-        GeCloudRESTResource.EXPECTATION_VALIDATION_RESULT: "result",
-        GeCloudRESTResource.PROFILER: "profiler",
-        GeCloudRESTResource.RENDERED_DATA_DOC: "rendered_data_doc",
-        # Chetan - 20220812 - SUITE_VALIDATION_RESULT is deprecated by GX Cloud and is to be removed upon migration of E2E tests


Removed CONTRACT and SUITE_VALIDATION_RESULT

…_expectations into m/_/clean_up_cloud_rest_resource_enum

cdkini · 2022-11-14T21:52:59Z

great_expectations/data_context/store/ge_cloud_store_backend.py

-SUPPORT_EMAIL = "support@greatexpectations.io"
-


Moved to cloud_constants.py

donaldheppner · 2022-11-15T00:04:11Z

great_expectations/data_context/data_context/base_data_context.py

@@ -1077,7 +1075,7 @@ def save_profiler(
        key: Union[GeCloudIdentifier, ConfigurationIdentifier]
        if self.ge_cloud_mode:
            key = GeCloudIdentifier(


Can we change the name of GeCloudIdentifier, too?

hmm as long as Cloud doesn't directly use this! I don't think they though but lemme peruse the Mercury code first

donaldheppner · 2022-11-15T00:04:55Z

great_expectations/data_context/types/resource_identifiers.py

@@ -203,7 +203,7 @@ def from_object(cls, validation_result):
 class GeCloudIdentifier(DataContextKey):


donaldheppner · 2022-11-15T00:05:20Z

great_expectations/rule_based_profiler/rule_based_profiler.py

@@ -1191,7 +1191,7 @@ def get_profiler(
        key: Union[GeCloudIdentifier, ConfigurationIdentifier]
        if ge_cloud_id:
            key = GeCloudIdentifier(
-                resource_type=GeCloudRESTResource.PROFILER, ge_cloud_id=ge_cloud_id
+                resource_type=GXCloudRESTResource.PROFILER, ge_cloud_id=ge_cloud_id


update the method parameter too? from ge_cloud_id to gx_cloud_id?

sounds good to me!

donaldheppner · 2022-11-15T00:06:03Z

tests/data_context/store/test_ge_cloud_store_backend.py

@@ -282,10 +282,10 @@ def test_list_keys(
 @pytest.mark.unit
 def test_remove_key(
    construct_ge_cloud_store_backend: Callable[
-        [GeCloudRESTResource], GeCloudStoreBackend
+        [GXCloudRESTResource], GeCloudStoreBackend


can we also rename the GeCloudStoreBackend?

that may break existing implementations due to our usage of instantiate_class_from_config - do we perhaps want to deprecate that name?

…_expectations into m/_/clean_up_cloud_rest_resource_enum

…b.com/great-expectations/great_expectations into m/_/clean_up_cloud_rest_resource_enum

* develop: [MAINTENANCE] Update column_reflection_fallback to also use schema name for Trino (#6350) [MAINTENANCE] Misc cleanup of GX Cloud helpers (#6352) [MAINTENANCE] Pin `mypy` to `0.990` (#6361) Subject: Support to include ID/PK in validation result for each row t… (#5876) [MAINTENANCE] Fix computed metrics type hint in ExecutionEngine.resolve_metrics() method (#6347) [MAINTENANCE] Refactor `usage_stats_opt_out` method in DataContext (#5339) Zep PostgresDatasource returns a list of batches. (#6341) [DOCS] Add `yarn snippet-check` command (#6351) [MAINTENANCE] Refactor out `termcolor` dependency (#6348) [MAINTENANCE] Move Cloud-specific enums to `cloud_constants.py` (#6349)

chore: remove unused fields in enum

1803b2f

github-actions bot added the core-team label Nov 14, 2022

chore: rename enums to GX

4bd3fbb

cdkini changed the title ~~[MAINTENANCE] Remove Contract and SuiteValidationResult from GeCloudRESTResource enum~~ [MAINTENANCE] Misc cleanup of GX Cloud helpers Nov 14, 2022

cdkini added 2 commits November 14, 2022 15:55

Merge branch 'develop' of https://github.com/great-expectations/great…

fa92d0d

…_expectations into m/_/clean_up_cloud_rest_resource_enum

chore: remove comments

7bd4d92

cdkini self-assigned this Nov 14, 2022

cdkini commented Nov 14, 2022

View reviewed changes

cdkini added 2 commits November 14, 2022 16:48

test: clear env vars before test run

b9a82fb

Merge branch 'develop' of https://github.com/great-expectations/great…

9d4007a

…_expectations into m/_/clean_up_cloud_rest_resource_enum

cdkini commented Nov 14, 2022

View reviewed changes

cdkini requested a review from a team November 14, 2022 21:54

donaldheppner reviewed Nov 15, 2022

View reviewed changes

Merge branch 'develop' of https://github.com/great-expectations/great…

0ce386c

…_expectations into m/_/clean_up_cloud_rest_resource_enum

Kilo59 approved these changes Nov 15, 2022

View reviewed changes

cdkini added 3 commits November 15, 2022 08:13

chore: rename a bunch of classes

9312a5e

chore: clean up mypy issues

2689905

chore: rename Id to ID

ef01c8c

cdkini enabled auto-merge (squash) November 15, 2022 14:41

github-actions bot and others added 4 commits November 15, 2022 15:04

Merge develop into m/_/clean_up_cloud_rest_resource_enum

8b670f5

Merge branch 'develop' of https://github.com/great-expectations/great…

e44918f

…_expectations into m/_/clean_up_cloud_rest_resource_enum

chore: clean up mypy

33ce8f3

Merge branch 'm/_/clean_up_cloud_rest_resource_enum' of https://githu…

81dbce8

…b.com/great-expectations/great_expectations into m/_/clean_up_cloud_rest_resource_enum

cdkini merged commit 9e16f81 into develop Nov 15, 2022

cdkini deleted the m/_/clean_up_cloud_rest_resource_enum branch November 15, 2022 15:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MAINTENANCE] Misc cleanup of GX Cloud helpers #6352

[MAINTENANCE] Misc cleanup of GX Cloud helpers #6352

cdkini commented Nov 14, 2022 •

edited

Loading

netlify bot commented Nov 14, 2022 •

edited

Loading

ghost commented Nov 14, 2022 •

edited by ghost

Loading

cdkini Nov 14, 2022

Kilo59 Nov 15, 2022

cdkini Nov 15, 2022

cdkini Nov 14, 2022

cdkini Nov 14, 2022

cdkini Nov 14, 2022

donaldheppner Nov 15, 2022

cdkini Nov 15, 2022

donaldheppner Nov 15, 2022

donaldheppner Nov 15, 2022

cdkini Nov 15, 2022

donaldheppner Nov 15, 2022

cdkini Nov 15, 2022

		@@ -203,7 +203,7 @@ def from_object(cls, validation_result):
		class GeCloudIdentifier(DataContextKey):

		SUPPORT_EMAIL = "support@greatexpectations.io"

[MAINTENANCE] Misc cleanup of GX Cloud helpers #6352

[MAINTENANCE] Misc cleanup of GX Cloud helpers #6352

Conversation

cdkini commented Nov 14, 2022 • edited Loading

Definition of Done

netlify bot commented Nov 14, 2022 • edited Loading

✅ Deploy Preview for niobium-lead-7998 ready!

ghost commented Nov 14, 2022 • edited by ghost Loading

Legend

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

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

cdkini commented Nov 14, 2022 •

edited

Loading

netlify bot commented Nov 14, 2022 •

edited

Loading

ghost commented Nov 14, 2022 •

edited by ghost

Loading