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: update kubernetes.mdx #14512

Merged
merged 5 commits into from
Nov 26, 2021
Merged

fix: update kubernetes.mdx #14512

merged 5 commits into from
Nov 26, 2021

Conversation

shicholas
Copy link
Contributor

SUMMARY

Update the Helm Kubernetes instruction to use bootstrapScript. In the helm values file, additionalRequirements currently referenced in the spec, does not exist.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API


```yaml
additionalRequirements:
Copy link
Contributor

Choose a reason for hiding this comment

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

additionalRequiremnts are no longer supported value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no this broke, please see #14188 and #13917. This PR may be a moot point if this chart is released up, see #14188 (comment) which is a comment from 14 days ago.

apt-get update -y &&\
apt-get install -y --no-install-recommends nano &&\
rm -rf /var/lib/apt/lists/*
pip install psycopg2==2.8.5 \
Copy link
Contributor

Choose a reason for hiding this comment

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

which versions are supported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are, I think the important thing here is to get a working demo, it should be up to the end-user to determine what packages they want to install in their own superset install.

@rusackas rusackas requested a review from craig-rueda August 1, 2021 01:52
@shicholas
Copy link
Contributor Author

sorry this took a while @mvoitko, I made your requested changes, but this PR may be moot per my comment above. Thanks for this awesome lib!

@villebro villebro closed this Nov 16, 2021
@villebro villebro reopened this Nov 16, 2021
@villebro villebro changed the title Update kubernetes.mdx fix: update kubernetes.mdx Nov 17, 2021
Copy link
Member

@dpgaspar dpgaspar left a comment

Choose a reason for hiding this comment

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

Makes sense to me, thank you for the PR. Just left a small comment

@@ -94,25 +94,18 @@ postgresql:

#### Dependencies

You can specify pip packages to be installed before startup, e.g. to install extra database drivers:
You can specify pip packages to be installed before startup. The following example installs the Big Query and Elasticsearch database drivers so that you can connect to those datasources in your Superset installation.
Copy link
Member

Choose a reason for hiding this comment

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

Let's include the comment on the values config file:

# Install additional packages and do any other bootstrap configuration in this script
# For production clusters it's recommended to build own image with this step done in CI

Since we should not encourage the use of a bootstrap script to install OS or python packages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

Copy link
Member

@dpgaspar dpgaspar left a comment

Choose a reason for hiding this comment

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

Last comment, but ready to go

docs/src/pages/docs/installation/kubernetes.mdx Outdated Show resolved Hide resolved
docs/src/pages/docs/installation/kubernetes.mdx Outdated Show resolved Hide resolved
@villebro villebro merged commit a304116 into apache:master Nov 26, 2021
@shicholas shicholas deleted the patch-1 branch November 27, 2021 04:50
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/S 🚢 1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants