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

improve helm chart #9346

Merged
merged 137 commits into from
Apr 22, 2020
Merged

improve helm chart #9346

merged 137 commits into from
Apr 22, 2020

Conversation

fbalicchia
Copy link
Contributor

CATEGORY

  • [x ] Enhancement (new features, refinement)

SUMMARY

Improve Superset chart

ADDITIONAL INFORMATION

Hi please review my PR, with this PR I've add possibility to

  • Configure superset_config.py in values using configmap
  • Enable/disable Postgres and Redis deploy
  • Possibility to add postStart hook for Superset config like create user etc.ect

This is for SupersetNode I will working soon on worker.

Thanks for your great work

@codecov-io
Copy link

Codecov Report

Merging #9346 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #9346   +/-   ##
=======================================
  Coverage   58.96%   58.96%           
=======================================
  Files         374      374           
  Lines       12137    12137           
  Branches     2992     2992           
=======================================
  Hits         7156     7156           
  Misses       4802     4802           
  Partials      179      179           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ccf21f6...d791822. Read the comment docs.

villebro and others added 21 commits March 22, 2020 22:31
* [config] Fixing GET_FEATURE_FLAGS_FUNC example

* Adding typing for GET_FEATURE_FLAGS_FUNC

* Update config.py
* [SQLLAB] add checkbox to control autocomplete

* autocomplete -> autocompleteEnabled

* fix defaultProps

* fix spec
* controls migrated

* linting
* migrating unique controls

* Lint ✨
* feat: [explore] don't save filters inherited from a dashboard

When navigating to explore from a dashboard context, the current
dashboard filter(s) are passed along to explore so that the context is
kept. So say you're filtering on "country=Romania", in your dashboard
and pivot to explore, that filter is still there and keep on exploring.

Now a common issue is that you'll want to make some tweak to your chart
that are unrelated to the filter, say toggling the legend off for
instance, and then save it. Now you back to your dashboard and even
though you started with an "all countries" dashboard, with a global
filter on country, now that one chart is stuck on "Romania". Typically
you notice this when filtering on something else, say "Italy" and then
that one chart now has two mutually exclusive filters, and show "No data".

Now, the fix is to flag the filter as "extra" (that's the not-so-good internal
name we use for these inherited filters) and make it clear that that
specific filter is special and won't be saved when saving the chart.

* fix build
* [charts] Refactor charts API using SIP-35

* [charts] Fix, copy pasta

* [charts] simplify
* [dataset] columns and metrics API (nested)

* [dataset] tests and validation

* [datasets] Fix, revert list field name to database_name
* Adding requirements-local.txt support

* Reverting package-lock.json
* Migrate Heatmap controls

* Lint
* [explore view] fix long query issue from Run in SQL LAB Button

* SQL Lab page needs to take the post form data, too

* fix variable names

* updated payload dict, rename hidden form

Co-authored-by: Jesse Yang <jesse.yang@airbnb.com>
* migrating controls

* Linting ✨

* oops... meant to pull this out in a conflict resolution.
Include superset-frontent/package.json in package because setup.py requires it
{{- end }}
lifecycle:
{{- if .Values.supersetNode.hooks.postStart }}
postStart:
Copy link
Member

Choose a reason for hiding this comment

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

Rather than using k8s hooks on the deployment level, you should instead spin a Job up which runs the init script. That way, users can safely tune their replica count up/down without re-triggering the init.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, Jobs in Kubernetes will run to completion once they are created. I' will make configurable enable/disable of init job and in the init containers of job image I'll wait for db Do you see any drawback ?

Choose a reason for hiding this comment

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

If a job fails another instance will be created until the job is completed or max retries reached. I think if the job fails because the database could not be reached and a new instance created after, this could be an alternative to adding logic in the container for waiting on db to be ready.

@@ -24,4 +24,14 @@ metadata:
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
data:
{{ (.Files.Glob "config/*").AsConfig | indent 2 }}
{{- $defaultConf := .Files.Get "files/superset_config.py" | nindent 4 }}
superset_config.py: |-
Copy link
Member

Choose a reason for hiding this comment

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

This config should really be in a secret. Although the current config has nothing "secret" in it, if a user were to try and use this in a production-like environment, they would likely have secrets in their configs

Copy link
Member

Choose a reason for hiding this comment

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

Also, pipe the outputs to tpl so that end users can use template syntax in their overrides


## (string) Script to execute after the client pod starts.
postStart: |-
#!/usr/bin/env bash
Copy link
Member

Choose a reason for hiding this comment

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

Maybe drop this in a script inside the files folder?

Grace Guo and others added 4 commits March 26, 2020 09:06
* [dashboard] handle markdown error

* localize error message, fix review comments.
Co-authored-by: John Bodley <john.bodley@airbnb.com>
rusackas and others added 5 commits April 17, 2020 11:40
* proto module

* caught a missed 'freq' unique control

* line_interpolation

* linting

* showLegend

* show_controls

* xAxisLabel

* bottomMargin

* x_ticks_layout

* missed one

* x_axis_format

* yLogScale

* y_axis_bounds

* linting

* nixing yarn lock

* x_axis_showminmax

* xAxisShowminmax control

* richTooltip

* linting, syntax fix

* show_bar_value, bar_stacked

* reduceXticks, yaxislabel

* left_margin, max_bubble_size, y_axis_showminmax

* show_labels

* send_time_range, y_axis_2_format, show_markers, order_bars

* nixing commented imports

* fake controls

* looking up actual controls for comparison.

* adding key to test setup

* controls inventory

* apache junk

* lint ✨

* ignore null controls

* fixing goofed up spread operation for xAxisFormat config

* lint ✨

* fixes for errors caused by <hr> element in filterbox controls

* fixing filter controls for 'druid_time_origin', 'granularity', 'granularity_sqla', 'time_grain_sqla'

* getControlsInventory -> getControlsForVizType

* further renaming of chartControlsInventory - > getControlsForVizType

Co-authored-by: David Aaron Suddjian <aasuddjian@gmail.com>
* fix: Add deprecated fields to QueryObject schema

* linting
Co-authored-by: John Bodley <john.bodley@airbnb.com>
* Re-enable the AnnotationLayerModelView read API

* Fix CI
dpgaspar and others added 10 commits April 20, 2020 15:48
Co-authored-by: John Bodley <john.bodley@airbnb.com>
* [Build] moves prettier check to separate script

* rename step: eslint -> lint
* Add documentation build to Github Actions

* Update requirements for documentation builds

* Minor optimization - only install requirements for documentation in documentation job
* add statsd to charts api

* update test for charts api

* [charts] add statsd asserts wrapper

* [charts] update api test

* removed white space
* [charts] adds new filters ui

* move null check to be more visible

* better filter lists and async filter functionality
@craig-rueda
Copy link
Member

@fbalicchia - can you rebase with master? Looks like GH Actions are stuck. I will merge once status is reporting properly...

@fbalicchia
Copy link
Contributor Author

fbalicchia commented Apr 22, 2020

Thanks, @craig-rueda rebase done. but I think that at moment of writing master is stuck too,
Please tell me if I can do something else

@craig-rueda craig-rueda merged commit d052f47 into apache:master Apr 22, 2020
@fbalicchia fbalicchia deleted the helm_improve branch April 25, 2020 06:38
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.37.0 labels Feb 28, 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/L 🚢 0.37.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.