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.

Sign up for GitHub

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

Jump to bottom

[MAINTENANCE] Add check to CloudDataContext to ensure using latest PyPI version #7753

Merged
merged 18 commits into from Apr 28, 2023
Merged

[MAINTENANCE] Add check to CloudDataContext to ensure using latest PyPI version #7753

merged 18 commits into from Apr 28, 2023

Conversation

cdkini
Copy link
Member

@cdkini cdkini commented Apr 27, 2023

Changes proposed in this pull request:

  • Cross reference with PyPI API to make sure we're using the latest GX

Definition of Done

  • My code follows the Great Expectations style guide
  • I have performed a self-review of my own code
  • 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.

@github-actions github-actions bot added the core label Apr 27, 2023
@netlify
Copy link

netlify bot commented Apr 27, 2023 •
edited

✅ Deploy Preview for niobium-lead-7998 ready!

Name Link
🔨 Latest commit 39c43c6
🔍 Latest deploy log https://app.netlify.com/sites/niobium-lead-7998/deploys/644c47149c60be0008cd5281
😎 Deploy Preview https://deploy-preview-7753--niobium-lead-7998.netlify.app
📱 Preview on mobile
Toggle QR Code...

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
Copy link

ghost commented Apr 27, 2023 •
edited by ghost

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

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

Kilo59
Kilo59 reviewed Apr 27, 2023
View reviewed changes
great_expectations/data_context/_version_checker.py
Comment on lines +38 to +41
def _get_latest_version_from_pypi(self) -> version.Version | None:
response_json: _PyPIPackageData | None = None
try:
response = requests.get(self._PYPI_GX_ENDPOINT)
Copy link
Member

@Kilo59 Kilo59 Apr 27, 2023 •
edited

Choose a reason for hiding this comment

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

It might be worth it to create a responses fixture mock for this.
Or... adding a check here to see if the code is being run from pytest.

https://stackoverflow.com/a/44595269/6304433

Copy link
Member Author

Choose a reason for hiding this comment

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

ooh might ask for some help here to make sure no other tests are impacted - will probably tackle tomorrow

Kilo59
Kilo59 approved these changes Apr 27, 2023
View reviewed changes
Copy link
Member

@Kilo59 Kilo59 left a comment

Choose a reason for hiding this comment

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

This will be a great debugging/onboarding/dev QOL improvement.

I think we should add a mock or conditional check to prevent this from becoming a source of flakey test failures.

@cdkini cdkini enabled auto-merge (squash) April 28, 2023 16:09
@cdkini cdkini disabled auto-merge April 28, 2023 18:14
@cdkini cdkini enabled auto-merge (squash) April 28, 2023 20:09
@cdkini cdkini merged commit 639b656 into develop Apr 28, 2023
47 checks passed
@cdkini cdkini deleted the m/lakitu-118/warn_if_not_on_latest_version_pypi branch April 28, 2023 23:19
Shinnnyshinshin added a commit that referenced this pull request Apr 28, 2023
@Shinnnyshinshin
Merge branch 'develop' into d/DX-469/spark-s3-deployment
aa987b6 
* develop:
  [MAINTENANCE] Add check to `CloudDataContext` to ensure using latest PyPI version (#7753)
  [RELEASE] 0.16.10 (#7774)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kilo59 Kilo59 Kilo59 approved these changes

Assignees
No one assigned
Labels
core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@cdkini @Kilo59