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

Engine registration and custom options #170

Closed
wants to merge 2 commits into from

Conversation

dcwatson
Copy link

This was originally part of #151. The short version is that this adds a register function to allow third-party database engines to be registered as URL schemes. It also lets you pass any DATABASES setting options through config/parse (see #116 for example).

@codecov
Copy link

codecov bot commented May 31, 2022

Codecov Report

Merging #170 (fcffaf1) into master (389febf) will increase coverage by 5.34%.
The diff coverage is 89.13%.

@@            Coverage Diff             @@
##           master     #170      +/-   ##
==========================================
+ Coverage   79.72%   85.07%   +5.34%     
==========================================
  Files           1        1              
  Lines          74       67       -7     
  Branches       14       19       +5     
==========================================
- Hits           59       57       -2     
+ Misses          9        6       -3     
+ Partials        6        4       -2     
Impacted Files Coverage Δ
dj_database_url.py 85.07% <89.13%> (+5.34%) ⬆️

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 389febf...fcffaf1. Read the comment docs.

@dcwatson
Copy link
Author

This is a breaking change in terms of the backends registered by default. I'm only registering the backends in Django itself, so no Redshift, Cockroach, MSSQL, etc. I could be convinced otherwise, but my thought was we probably want to get out of the business of blessing third-party database engines and just make it as easy as possible for people to use whatever they like.

I should mention, my original intention was to fork the project, so I hadn't written any documentation for this yet. I'm (obviously) inclined to merge this, but would love a second set of eyes on it. I'll also work on corresponding updates to the README to document the registration process, and the breaking change above.

@mattseymour
Copy link
Contributor

@dcwatson thanks for the submission, we have had two very similar yet competing PRs come in within a couple of hours of each other. Can I direct you to #171 and see if the changes you made can work along side this PR.

📓 I have put a similar message on #171

@dcwatson
Copy link
Author

dcwatson commented Jun 1, 2022

I'll close this in favor of #170. I still think dropping pre-2.0 Django support and not automatically registering third-party engines are worthy goals, but can be separate issues.

@dcwatson dcwatson closed this Jun 1, 2022
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.

2 participants