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

chore: isolate examples database by default #25003

Merged
merged 3 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ services:
- "127.0.0.1:5432:5432"
volumes:
- db_home:/var/lib/postgresql/data
- ./docker/docker-entrypoint-initdb.d:/docker-entrypoint-initdb.d

superset:
env_file: docker/.env
Expand Down
6 changes: 6 additions & 0 deletions docker/.env
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ DATABASE_HOST=db
DATABASE_PASSWORD=superset
DATABASE_USER=superset

EXAMPLES_DB=examples
EXAMPLES_HOST=db
EXAMPLES_USER=examples
EXAMPLES_PASSWORD=examples
EXAMPLES_PORT=5432

# database engine specific environment variables
# change the below if you prefer another database engine
DATABASE_PORT=5432
Expand Down
15 changes: 15 additions & 0 deletions docker/docker-entrypoint-initdb.d/examples-init.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# ------------------------------------------------------------------------
# Creates the examples database and repective user. This database location
# and access credentials are defined on the environment variables
# ------------------------------------------------------------------------
set -e

psql -v ON_ERROR_STOP=1 --username "${POSTGRES_USER}" <<-EOSQL
CREATE USER ${EXAMPLES_USER} WITH PASSWORD '${EXAMPLES_PASSWORD}';
CREATE DATABASE ${EXAMPLES_DB};
GRANT ALL PRIVILEGES ON DATABASE ${EXAMPLES_DB} TO ${EXAMPLES_USER};
EOSQL

psql -v ON_ERROR_STOP=1 --username "${POSTGRES_USER}" -d "${EXAMPLES_DB}" <<-EOSQL
GRANT ALL ON SCHEMA public TO ${EXAMPLES_USER};
EOSQL
24 changes: 17 additions & 7 deletions docker/pythonpath_dev/superset_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,24 @@ def get_env_variable(var_name: str, default: Optional[str] = None) -> str:
DATABASE_PORT = get_env_variable("DATABASE_PORT")
DATABASE_DB = get_env_variable("DATABASE_DB")

EXAMPLES_USER = get_env_variable("EXAMPLES_USER")
EXAMPLES_PASSWORD = get_env_variable("EXAMPLES_PASSWORD")
EXAMPLES_HOST = get_env_variable("EXAMPLES_HOST")
EXAMPLES_PORT = get_env_variable("EXAMPLES_PORT")
EXAMPLES_DB = get_env_variable("EXAMPLES_DB")


# The SQLAlchemy connection string.
SQLALCHEMY_DATABASE_URI = "{}://{}:{}@{}:{}/{}".format(
DATABASE_DIALECT,
DATABASE_USER,
DATABASE_PASSWORD,
DATABASE_HOST,
DATABASE_PORT,
DATABASE_DB,
SQLALCHEMY_DATABASE_URI = (
f"{DATABASE_DIALECT}://"
f"{DATABASE_USER}:{DATABASE_PASSWORD}@"
f"{DATABASE_HOST}:{DATABASE_PORT}/{DATABASE_DB}"
)

SQLALCHEMY_EXAMPLES_URI = (
f"{DATABASE_DIALECT}://"
f"{EXAMPLES_USER}:{EXAMPLES_PASSWORD}@"
f"{EXAMPLES_HOST}:{EXAMPLES_PORT}/{EXAMPLES_DB}"
)

REDIS_HOST = get_env_variable("REDIS_HOST")
Expand Down
2 changes: 1 addition & 1 deletion superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1452,7 +1452,7 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument

# URI to database storing the example data, points to
# SQLALCHEMY_DATABASE_URI by default if set to `None`
SQLALCHEMY_EXAMPLES_URI = None
SQLALCHEMY_EXAMPLES_URI = "sqlite:///" + os.path.join(DATA_DIR, "examples.db")
Copy link
Member

Choose a reason for hiding this comment

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

@dpgaspar are we supporting sqlite as a valid examples db in production?

Copy link
Member Author

Choose a reason for hiding this comment

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

same as before since our default for the metadata db is also sqlite https://github.com/apache/superset/blob/master/superset/config.py#L187
The difference is before this PR both schemas were on the same sqlite db

Copy link
Member

Choose a reason for hiding this comment

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

We have an approved SIP to drop support for SQL Lite. I've added a ticket to the 4.0 board to ensure we execute this in the next major release.

Copy link
Member

Choose a reason for hiding this comment

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

We do discourage people to use sqlite, though, so I'm wondering if we should put something in UPDATING.md that encourage people to change this value to a different db type?


# Optional prefix to be added to all static asset paths when rendering the UI.
# This is useful for hosting assets in an external CDN, for example
Expand Down
6 changes: 1 addition & 5 deletions superset/utils/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,7 @@ def get_or_create_db(


def get_example_database() -> Database:
db_uri = (
current_app.config.get("SQLALCHEMY_EXAMPLES_URI")
or current_app.config["SQLALCHEMY_DATABASE_URI"]
)
return get_or_create_db("examples", db_uri)
return get_or_create_db("examples", current_app.config["SQLALCHEMY_EXAMPLES_URI"])
Copy link
Member

Choose a reason for hiding this comment

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

Regarding #23399, should we have the examples DB have DML on and with file uploads enabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

left a comment on #23399, no strong opinions about allowing DML/upload on the examples db, yet I do see this as a readonly db



def get_main_database() -> Database:
Expand Down
Loading