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

[Helm] Automate datasource import #10771

Merged
merged 3 commits into from
Feb 2, 2021

Conversation

ejianu
Copy link
Contributor

@ejianu ejianu commented Sep 2, 2020

SUMMARY

Helm chart change: Added 2 possible properties which will essentially automate the creation of database connections in superset.

Changes in the values file:
extraConfigs to which we can pass a yaml file that we exported previously, that defines a datasource.
extraConfigMountPath location at which this file will be

Changes in the chart:

Added a configMap template file that only creates the ConfigMap if the extraConfigs is set.
In the init-job file, the new "extra-config" volume is mounted only if extraConfigs is set.

In this way, the original approach should not be affected.

TEST PLAN

  1. Make sure you create a datasource in the interface and export it to yaml, then delete it and check it was deleted.
  2. In the values file set the yaml you exported, this is an example, but any datasource exported yaml should do:
extraConfigs: 
  datasources.yaml: | 
      databases:
      - allow_csv_upload: true
        allow_ctas: true
        allow_cvas: true
        database_name: presto
        extra: "{\r\n    \"metadata_params\": {},\r\n    \"engine_params\": {},\r\n    \"\
          metadata_cache_timeout\": {},\r\n    \"schemas_allowed_for_csv_upload\": []\r\n\
          }"
        sqlalchemy_uri: presto://presto.whatever
        tables: []
  1. set the following property, the datasource file will be mounted at some location:
    extraConfigMountPath: "/app/configs"

  2. under the Init job configuration in the init script add a command that imports the file we defined previously:
    echo "Initializing connection.... "
    superset import_datasources -p {{ .Values.extraConfigMountPath }}/datasources.yaml

  3. Check if the datasource was imported correctly.

Copy link
Member

@craig-rueda craig-rueda left a comment

Choose a reason for hiding this comment

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

One quick thing, otherwise LGTM

data:
{{- range $path, $config := .Values.extraConfigs }}
{{ $path }}: |
{{ $config | indent 4 -}}
Copy link
Member

Choose a reason for hiding this comment

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

Can you pipe the val to tpl?

So, {{ tpl $config . | indent 4 -}}

@ejianu
Copy link
Contributor Author

ejianu commented Sep 3, 2020

Changes made :)

@willbarrett
Copy link
Member

Please trim trailing whitespace to fix the precommit hook, after that should be good to merge.

@@ -238,4 +253,4 @@ nodeSelector: {}

tolerations: []

affinity: {}
affinity: {}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
affinity: {}
affinity: {}

{{ $path }}: |
{{ tpl $config . | indent 4 -}}
{{- end -}}
{{- end -}}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{{- end -}}
{{- end -}}

@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

@stale stale bot added the inactive Inactive for >= 30 days label Dec 25, 2020
@andreas-eberle
Copy link
Contributor

andreas-eberle commented Feb 1, 2021

What is the status on these changes? I'd really like to see them integrated.

@stale stale bot removed the inactive Inactive for >= 30 days label Feb 1, 2021
@willbarrett willbarrett merged commit 388edbf into apache:master Feb 2, 2021
amitmiran137 pushed a commit to nielsen-oss/superset that referenced this pull request Feb 3, 2021
* master: (23 commits)
  feat(explore): clear search on dataset change (apache#12909)
  chore: remove SIP-38 feature flag (apache#12894)
  fix: Config for dataset health check (apache#12906)
  fix(chart): allow null for most query object props (apache#12905)
  feat: add separate endpoint to fetch function names for autocomplete (apache#12840)
  chore: add required review on master (apache#12694)
  fix: comment typo (apache#12898)
  Migrates Radio component from Bootstrap to AntD. (apache#12738)
  fix: allow users to reset their passwords (apache#12886)
  fix(explore): missing select when groupby without metrics (apache#12890)
  refactor: dbapi exception mapping for dbapi's (apache#12869)
  feat(style-theme): add support for custom superset themes (apache#12858)
  chore(lint): fix pre-commit error (apache#12884)
  refactor(color-schemes): refactor setting of color schemes (apache#12857)
  feat(native-filters): Add defaultValue for Native filters modal (apache#12199)
  feat(release): add github token to changelog script (apache#12872)
  fix(menu): always show settings dropdown (apache#12877)
  Migrates Label component from Bootstrap to AntD. (apache#12774)
  [Helm] Automate datasource import (apache#10771)
  build: Skip loading example data from configs in CI (apache#12610)
  ...
@zhmurko
Copy link

zhmurko commented Apr 15, 2021

if someone is looking why import with extraConfigs does not work, just add into initscript a command:

        echo "Initializing connection.... "
        superset import_datasources -p {{ .Values.extraConfigMountPath }}/datasources-init.yaml

this step is not present in master branch at this moment.

@jawabuu
Copy link
Contributor

jawabuu commented Apr 22, 2021

@ejianu @craig-rueda I have noticed that databases imported in this manner do not mask the password value like the ones added in the UI. Is this expected?

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Feb 19, 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/M 🚢 1.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants