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

Adding hook for external password store for databases #3436

Merged
merged 1 commit into from
Sep 14, 2017

Conversation

fabianmenges
Copy link
Contributor

This allows you to write a plugin for an external password store for you sql passwords.

My employer has a strict policy that all credentials are managed by a single system, this now allowing us to store SQL passwords inside of the Superset meta-database .

@coveralls
Copy link

coveralls commented Sep 8, 2017

Coverage Status

Coverage decreased (-0.001%) to 69.117% when pulling c33355c1997dad1c2e172b8a99ec3b8f13e48a2c on tc-dc:fmenges/external_password_store into 3dfdde1 on apache:master.

@@ -60,6 +60,15 @@
# SQLALCHEMY_DATABASE_URI = 'mysql://myapp@localhost/myapp'
# SQLALCHEMY_DATABASE_URI = 'postgresql://root:password@localhost/myapp'

# In order to hook up a custom password store for all SQLACHEMY connections
Copy link
Contributor

Choose a reason for hiding this comment

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

Better add documentation to documentation instead of config files. You can just set
SQLALCHEMY_CUSTOM_PASSWORD_STORE = False here

Copy link
Member

@mistercrunch mistercrunch left a comment

Choose a reason for hiding this comment

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

Agreed with @xrmx , please add a note in installation.rst (and rebase)

@fabianmenges fabianmenges force-pushed the fmenges/external_password_store branch 2 times, most recently from f4140a0 to b49827d Compare September 12, 2017 14:48
@coveralls
Copy link

coveralls commented Sep 12, 2017

Coverage Status

Coverage decreased (-0.001%) to 69.117% when pulling b49827d90e4bee62a7abe14a8975e6cbed24c2be on tc-dc:fmenges/external_password_store into 147c12d on apache:master.

@coveralls
Copy link

coveralls commented Sep 12, 2017

Coverage Status

Coverage decreased (-0.001%) to 69.117% when pulling b49827d90e4bee62a7abe14a8975e6cbed24c2be on tc-dc:fmenges/external_password_store into 147c12d on apache:master.

@coveralls
Copy link

coveralls commented Sep 12, 2017

Coverage Status

Coverage decreased (-0.09%) to 69.027% when pulling 95a52d3b3b5e30dc943cf015cf3b0e033e6d7ec9 on tc-dc:fmenges/external_password_store into 8223729 on apache:master.

@coveralls
Copy link

coveralls commented Sep 13, 2017

Coverage Status

Coverage increased (+0.01%) to 69.133% when pulling 133d3d3 on tc-dc:fmenges/external_password_store into 8223729 on apache:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 69.133% when pulling 133d3d3 on tc-dc:fmenges/external_password_store into 8223729 on apache:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 69.133% when pulling 133d3d3 on tc-dc:fmenges/external_password_store into 8223729 on apache:master.


database.custom_password_store = custom_password_store
conn = sqla.engine.url.make_url(database.sqlalchemy_uri_decrypted)
if conn_pre.password:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sqlite does not support passwords, so this test would fail otherwise.

@mistercrunch mistercrunch merged commit fdee06b into apache:master Sep 14, 2017
@fabianmenges fabianmenges deleted the fmenges/external_password_store branch September 19, 2017 18:32
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.20.0 labels Feb 27, 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 🚢 0.20.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants