Superset deployment #3715

bendnorman · 2024-07-11T21:01:34Z

Overview

This PR contains superset configuration and cloud deployment changes for our data exploration tool.

Testing

How did you make sure this worked? How can a reviewer verify this?

To-do list

Give feedback

If updating analyses or data processing functions: make sure to update or write data validation tests (e.g., test_minmax_rows())
Update the release notes: reference the PR and related issues.
Ensure docs build, unit & integration tests, and test coverage pass locally with make pytest-coverage (otherwise the merge queue may reject your PR)
Review the PR yourself and call out any questions or issues you have
For minor ETL changes or data additions, once make pytest-coverage passes, make sure you have a fresh full PUDL DB downloaded locally, materialize new/changed assets and all their downstream assets and run relevant data validation tests using pytest and --live-dbs.
For significant ETL, data coverage or analysis changes, once make pytest-coverage passes, ensure the full ETL runs locally and run data validation tests using make pytest-validate (a ~10 hour run). If you can't run this locally, run the build-deploy-pudl GitHub Action (or ask someone with permissions to). Then, check the logs on the #pudl-deployments Slack channel or gs://builds.catalyst.coop.
Options

… database connection yet

These changes have not been applied! I couldn't figure out how mount the bucket as a volume in cloud run using tf so I used the GUI. Now, when I run terraform plan tf wants to change a bunch of attributes for the cloud run instance. This commit also changes the default superset registration user

jdangerx

Driveby comments since I was looking at a bunch of tickets. Glad you got that role stuff sorted out - ping me whenever you want a more in-depth review!

superset/superset_config.py

terraform/main.tf

…build script

bendnorman · 2024-08-14T01:20:00Z

superset/Dockerfile

@@ -0,0 +1,14 @@
+# hadolint ignore=DL3006
+FROM apache/superset


@jdangerx should we pin the base image version? If so, can we set up dependabot to keep the base image and the requirements.txt file up to date?

Yes, we should pin the base image version since a superset upgrade might require a DB migration as well: https://superset.apache.org/docs/installation/upgrading-superset/

I think we can set up Dependabot to make PRs to keep the docker image up to date by adding a docker block to the updates in dependabot.yml, and then labeling the Dockerfile source: https://github.com/dependabot-fixtures/docker-with-source/blob/main/Dockerfile

And I think we can add a pip block for requirements.txt.

I think both of those can be in a follow-up PR, though, and we should set some sort of reminder later to check to see if the dependabot config actually worked...

bendnorman · 2024-08-14T01:21:15Z

superset/README.md

+If this is the first time running superset locally or you recently ran `docker compose down` you'll need to run the commands in `setup.sh`.
+
+## Making changes to the production deployment
+TODO: instructions on how to connect to Cloud SQL


I need to figure out how to use the Cloud Auth Proxy to make changes to the production database. Also, how can we protect against people making changes to the production database when they are just experimenting with the local deployment?

How did you make changes to the Cloud SQL database for the initial deploy?

I think we can use the Docker version of the Cloud SQL Auth Proxy in a mutate-prod docker compose file. That would just be the same as our dev docker-compose, but with the Cloud SQL proxy standing in for the postgres service. Then executing superset db-upgrade or whatever, within the pudl-superset docker-compose service, would point at prod Cloud SQL.

We'd need to make a new service account to give that Cloud SQL Auth Proxy. To stop people from accidentally changing the prod DB, we could restrict the ability to create a key for that SA to only a subset of Catalyst. So it'd require a fair amount of effort/oversight to be able to make a change to prod DB.

bendnorman · 2024-08-14T01:23:05Z

superset/README.md

+## Making changes to the production deployment
+TODO: instructions on how to connect to Cloud SQL
+
+## Deploy to Cloud Run


What should the deployment flow be for superset? I can think of a few reasons to trigger a new deployment:
When do we want to update superset

package in requirements.txt is updated

the base image is updated

we make a change to superset_config.py

I think we’ll probably want to redeploy superset when there is new data because changes to tables might break shared dashboards. Also, should superset point at nightly or stable?

I think redeploying Superset for nightly builds is a good way to test that our deployment infrastructure still works, and has the benefit of catching all these other changes too - so long as the cloud build doesn't cost a ton I think that's the easiest way forward.

I also think that eventually we probably want both nightly and stable to be on Superset - we can default to using stable, but have the option to connect to nightly in the database selector if people want.

bendnorman · 2024-08-14T01:26:07Z

superset/docker-compose.yml

+      - "8080:8088"
+    volumes:
+      # - {path to your pudl.duckdb file}:/app/pudl.duckdb
+      - ./roles.json:/app/roles.json


Do we want to try to track the role definitions in git so our local deployments can use the roles we're using in production?

Yeah, that makes a ton of sense to me :)

bendnorman · 2024-08-14T01:27:29Z

superset/superset_config.py

+def FLASK_APP_MUTATOR(app: Flask) -> None:  # noqa: N802
+    """Superset function that allows you to configure the Flask app.
+
+    Args:
+        app: The Flask app instance
+    """
+    app.config.update(
+        PREFERRED_URL_SCHEME="https",
+    )


Unfortunately, this didn't resolve the HTTP redirect issue. I think the issue might be related to the auth0 Oauth development tokens we're using.

Hmm, nothing in that list looks suspicious to me, I'd love to see the authentication logs and see if there are specific errors showing up, or what the callback URL is being parsed as, etc.

bendnorman · 2024-08-14T01:30:55Z

terraform/main.tf

@@ -102,3 +102,297 @@ resource "google_storage_bucket_iam_binding" "binding" {
    "user:[email protected]",
  ]
 }
+


I'd like to learn more about how to structure terraform projects. It'd be helpful to isolate the superset setup in a separate file or something so we can easily reuse it for other projects like the pudl usage metrics repo.

It's pretty straightforward - terraform mashes all the *.tf files into one thing before parsing, so you can split however you want. Check out this article: https://build5nines.com/terraform-split-main-tf-into-seperate-files/

jdangerx

Thanks for wrangling all this! There's definitely bits to change but also it seems like it works.

The main blocking thing is just pinning the docker image - once that's done I'm happy to merge to main.

It's probably also a good idea to get people to run their own Auth0 apps instead of using the production one for development.

Some improvements/follow-ups that could be separate PRs:

triggering redeploys on nightly build
define a new SA specifically for Superset and give that all the accesses it needs - probably refactoring to use for_each along the way.
get a docker-compose setup for pointing at production

jdangerx · 2024-08-14T14:48:51Z

superset/Dockerfile

@@ -0,0 +1,14 @@
+# hadolint ignore=DL3006
+FROM apache/superset


Yes, we should pin the base image version since a superset upgrade might require a DB migration as well: https://superset.apache.org/docs/installation/upgrading-superset/

I think we can set up Dependabot to make PRs to keep the docker image up to date by adding a docker block to the updates in dependabot.yml, and then labeling the Dockerfile source: https://github.com/dependabot-fixtures/docker-with-source/blob/main/Dockerfile

And I think we can add a pip block for requirements.txt.

I think both of those can be in a follow-up PR, though, and we should set some sort of reminder later to check to see if the dependabot config actually worked...

superset/README.md

jdangerx · 2024-08-14T15:09:39Z

superset/README.md

+If this is the first time running superset locally or you recently ran `docker compose down` you'll need to run the commands in `setup.sh`.
+
+## Making changes to the production deployment
+TODO: instructions on how to connect to Cloud SQL


How did you make changes to the Cloud SQL database for the initial deploy?

I think we can use the Docker version of the Cloud SQL Auth Proxy in a mutate-prod docker compose file. That would just be the same as our dev docker-compose, but with the Cloud SQL proxy standing in for the postgres service. Then executing superset db-upgrade or whatever, within the pudl-superset docker-compose service, would point at prod Cloud SQL.

We'd need to make a new service account to give that Cloud SQL Auth Proxy. To stop people from accidentally changing the prod DB, we could restrict the ability to create a key for that SA to only a subset of Catalyst. So it'd require a fair amount of effort/oversight to be able to make a change to prod DB.

jdangerx · 2024-08-14T16:28:03Z

superset/superset_config.py

+def FLASK_APP_MUTATOR(app: Flask) -> None:  # noqa: N802
+    """Superset function that allows you to configure the Flask app.
+
+    Args:
+        app: The Flask app instance
+    """
+    app.config.update(
+        PREFERRED_URL_SCHEME="https",
+    )


Hmm, nothing in that list looks suspicious to me, I'd love to see the authentication logs and see if there are specific errors showing up, or what the callback URL is being parsed as, etc.

jdangerx · 2024-08-14T16:30:04Z

terraform/main.tf

@@ -102,3 +102,297 @@ resource "google_storage_bucket_iam_binding" "binding" {
    "user:[email protected]",
  ]
 }
+


It's pretty straightforward - terraform mashes all the *.tf files into one thing before parsing, so you can split however you want. Check out this article: https://build5nines.com/terraform-split-main-tf-into-seperate-files/

jdangerx · 2024-08-14T17:00:05Z

terraform/main.tf

+  }
+}
+
+resource "google_sql_database_instance" "postgres_pvp_instance_name" {


What does pvp mean here?

I grabbed this terraform code from the google docs and forgot to rename the terraform resource. Is it too late to rename it?

If you rename it, it will delete the resource and create a new one. So it sort of depends on how hard it is to re-initialize the state.

jdangerx · 2024-08-14T20:48:40Z

terraform/main.tf

+  deletion_protection = true
+}
+
+resource "google_secret_manager_secret" "superset_database_username" {


Hooray! I love that you're using the secret manager to manage secrets, it makes my shriveled little security heart sing.

It's a little heavyweight to define all this boilerplate just to say "there are these secrets, here are their names, and this service account has access to them." We can use the for_each syntax of Terraform to drastically reduce that.

Note that if you use for_each it will change all the resource names in Terraform, so it will want to recreate everything -_- which means you'll have to manually re-populate the secrets.

Ah good to know! I tackle it when I create a new service account.

jdangerx · 2024-08-14T20:49:53Z

terraform/main.tf

+resource "google_secret_manager_secret_iam_member" "superset_database_username_compute_iam" {
+  secret_id = google_secret_manager_secret.superset_database_username.id
+  role      = "roles/secretmanager.secretAccessor"
+  member    = "serviceAccount:[email protected]"


I think it makes sense to make a new service account that's only for Superset instead of using the default Compute Engine SA. Then it's easier to restrict access to the prod database, which makes it harder to accidentally blow it up.

Ah yes it sure does. I'll create an issue for this.

Co-authored-by: Dazhong Xia <[email protected]>

…ker compose env vars

bendnorman · 2024-08-16T00:05:56Z

Thanks for all the great feedback! I pinned the docker base image version, updated auth0 env var instructions and set docker compose env var defaults. I also created some draft issues that I'll flesh out tomorrow.

jdangerx · 2024-08-16T16:56:13Z

You're hitting a bunch of this error in CI:

ERROR test/integration/glue_test.py::test_unmapped_utils_eia - TypeError: ForwardRef._evaluate() missing 1 required keyword-only argument: 'recursive_guard'

Which is related to dagster / python version incompatibilities: dagster-io/dagster#22985

bendnorman added 2 commits July 1, 2024 20:38

Mostly working auth0 setup

ada360a

Play with combining permissions

58ea963

bendnorman self-assigned this Jul 11, 2024

bendnorman added datasette Issues related the accessing PUDL data via Datasette. duckdb Issues referring to duckdb, the embedded OLAP database superset labels Jul 11, 2024

bendnorman added 4 commits July 11, 2024 13:46

Enable working auth0 set up

60f9772

Add import GammaSQLLab role

63658f6

Functioning superset with auth0 registration and postgres backend. No…

28a8cc4

… database connection yet

bendnorman linked an issue Jul 25, 2024 that may be closed by this pull request

Deploy superset #3703

Closed

bendnorman added 2 commits August 1, 2024 10:31

Add superset cloud run gcs mount to terraform

9f59337

Use pg8000 and sqlalchemy URL to connect to cloud sql

fd9c467

jdangerx reviewed Aug 7, 2024

View reviewed changes

superset/superset_config.py Outdated Show resolved Hide resolved

terraform/main.tf Show resolved Hide resolved

bendnorman added 3 commits August 8, 2024 14:13

Switch back to using connection string for superset backend

cb6c089

Add readme, requirements file, cloud run deployment command to cloud …

b52ab53

…build script

Merge branch 'main' into init-superset

d967142

bendnorman commented Aug 14, 2024

View reviewed changes

bendnorman marked this pull request as ready for review August 14, 2024 01:28

bendnorman commented Aug 14, 2024

View reviewed changes

jdangerx self-requested a review August 14, 2024 14:36

jdangerx requested changes Aug 14, 2024

View reviewed changes

bendnorman and others added 4 commits August 15, 2024 14:36

Update superset/README.md

ac5144d

Co-authored-by: Dazhong Xia <[email protected]>

Update superset/README.md

642fd66

Co-authored-by: Dazhong Xia <[email protected]>

Merge branch 'main' into init-superset

30dfb04

Add auth0 instructions to readme, add default values for superset doc…

75e6875

…ker compose env vars

Pin superset base image version

1039512

jdangerx approved these changes Aug 16, 2024

View reviewed changes

bendnorman added this pull request to the merge queue Aug 16, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 16, 2024

Update conda lock files

fbbfb2b

bendnorman added this pull request to the merge queue Aug 16, 2024

Merged via the queue into main with commit f257fc8 Aug 16, 2024
17 checks passed

bendnorman deleted the init-superset branch August 16, 2024 20:02

bendnorman mentioned this pull request Sep 17, 2024

Create Usage metrics dashboard #1465

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Superset deployment #3715

Superset deployment #3715

bendnorman commented Jul 11, 2024 •

edited

Loading

To-do list

jdangerx left a comment •

edited

Loading

bendnorman Aug 14, 2024

jdangerx Aug 14, 2024

bendnorman Aug 14, 2024

jdangerx Aug 14, 2024

bendnorman Aug 14, 2024

jdangerx Aug 14, 2024

bendnorman Aug 14, 2024

jdangerx Aug 14, 2024

bendnorman Aug 14, 2024

jdangerx Aug 14, 2024

bendnorman Aug 14, 2024

jdangerx Aug 14, 2024

jdangerx left a comment

jdangerx Aug 14, 2024

jdangerx Aug 14, 2024

jdangerx Aug 14, 2024

jdangerx Aug 14, 2024

jdangerx Aug 14, 2024

bendnorman Aug 15, 2024

jdangerx Aug 16, 2024

jdangerx Aug 14, 2024 •

edited

Loading

bendnorman Aug 15, 2024 •

edited

Loading

jdangerx Aug 14, 2024

bendnorman Aug 15, 2024

bendnorman commented Aug 16, 2024

jdangerx commented Aug 16, 2024

		@@ -0,0 +1,14 @@
		# hadolint ignore=DL3006
		FROM apache/superset

Superset deployment #3715

Superset deployment #3715

Conversation

bendnorman commented Jul 11, 2024 • edited Loading

Overview

Testing

To-do list

jdangerx left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdangerx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdangerx Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

bendnorman Aug 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bendnorman commented Aug 16, 2024

jdangerx commented Aug 16, 2024

bendnorman commented Jul 11, 2024 •

edited

Loading

jdangerx left a comment •

edited

Loading

jdangerx Aug 14, 2024 •

edited

Loading

bendnorman Aug 15, 2024 •

edited

Loading