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

Database parameters #3450

Merged
merged 10 commits into from Jun 7, 2023
Merged

Database parameters #3450

merged 10 commits into from Jun 7, 2023

Conversation

ewdurbin
Copy link
Contributor

@ewdurbin ewdurbin commented Jun 4, 2023

Closes #1791

Code Changes

  • Implement a database parameters configuration that is urlencoded as a querystring for the constructed URIs

Steps to Confirm

  • Configure some optional database parameters like sslmode or sslrootcert and note that they are associated with the URIs and if your database doesn't support TLS (like in dev/test) things go 💥

Pre-Merge Checklist

Description Of Changes

This implements the ability for users to configure database connection parameters to support things like TLS connections with full validation.

I chose the most direct and flexible option in issue #1791, as tweaking these means a user generally knows what they want very specifically.

@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Patch coverage: 76.47% and project coverage change: -0.01 ⚠️

Comparison is base (741bbce) 87.14% compared to head (99045e2) 87.13%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3450      +/-   ##
==========================================
- Coverage   87.14%   87.13%   -0.01%     
==========================================
  Files         312      312              
  Lines       18761    18774      +13     
  Branches     2390     2392       +2     
==========================================
+ Hits        16349    16359      +10     
- Misses       1988     1991       +3     
  Partials      424      424              
Impacted Files Coverage Δ
src/fides/api/ctl/database/session.py 78.94% <50.00%> (-21.06%) ⬇️
src/fides/core/config/database_settings.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ThomasLaPiana ThomasLaPiana merged commit 29b3f6c into ethyca:main Jun 7, 2023
36 of 37 checks passed
@ThomasLaPiana
Copy link
Contributor

@RobertKeyser want to try using these now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Configuration Option to Pass Additional Parameters to the DB Connection String
2 participants