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.

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

fix superset docs and pin conflicting dependency #1169

Merged
merged 3 commits into from
Feb 15, 2023

Conversation

pnadolny13
Copy link
Contributor

@pnadolny13 pnadolny13 commented Feb 14, 2023

Related to meltano/superset-ext#8

  • The docs were outdated for the EDK variant using superset_ext which isnt what the extension expects anymore
  • Theres an issue in the superset dependencies that causes an error unless we explicitly pin a version of cryptography also see MDS in a box where the same thing was done. I updated for both of these variants
  • Set the EDK variant to default

@netlify
Copy link

netlify bot commented Feb 14, 2023

Deploy Preview for meltano-hub ready!

Name Link
🔨 Latest commit a53203b
🔍 Latest deploy log https://app.netlify.com/sites/meltano-hub/deploys/63ec0b4a2eb6410008ac5c6e
😎 Deploy Preview https://deploy-preview-1169--meltano-hub.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.

@pnadolny13 pnadolny13 temporarily deployed to preview February 14, 2023 22:29 — with GitHub Actions Inactive
@pnadolny13 pnadolny13 changed the title fix superset extension docs and pin conflicting dependency fix superset docs and pin conflicting dependency Feb 14, 2023
@pnadolny13 pnadolny13 temporarily deployed to preview February 14, 2023 22:32 — with GitHub Actions Inactive
@cwegener
Copy link

@pnadolny13 The cryptography version issue fix is planned to be included in the 2.0.2 release.

@pnadolny13
Copy link
Contributor Author

pnadolny13 commented Feb 15, 2023

@pnadolny13 The cryptography version issue fix is planned to be included in the 2.0.2 release.

@cwegener thanks for that! I went looking and found that the release should be out next week and you linked to apache/superset#22744 that fixes the problem.

I'd vote to merge this ASAP, then open an issue to bump to 2.0.2 + remove the cryptography pin after the release next week.

UPDATE: issue created #1172

Copy link
Collaborator

@tayloramurphy tayloramurphy left a comment

Choose a reason for hiding this comment

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

I'm still not a fan of having this as the Meltano variant, but it's not critical we solve for that problem right now. Switching to the EDK as default makes sense to me.

@pnadolny13
Copy link
Contributor Author

pnadolny13 commented Feb 15, 2023

I'm still not a fan of having this as the Meltano variant, but it's not critical we solve for that problem right now. Switching to the EDK as default makes sense to me.

@tayloramurphy yeah thats fair, cross linking back to our previous discussion on this #1135 (comment). I'll open a discussion so it doesn't get lost. #1171

@pnadolny13 pnadolny13 merged commit ea01635 into main Feb 15, 2023
@pnadolny13 pnadolny13 deleted the bugfix/superset_extension branch February 15, 2023 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants