-
Notifications
You must be signed in to change notification settings - Fork 21
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
Improve DX using connections. #159
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit dbff7f0.
This reverts commit 397cce2.
…ic-redirect into split-connections
This reverts commit 17c7365.
People can read the up-to-date documentation instead.
vluijkx
changed the title
Improve developer DX using connections.
Improve DX using connections.
Jan 8, 2024
Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi again! 😅
We need a setup in which errors are kept in the default sqlite store, while redirects are stored in the main Laravel database. Doing this caused all sorts of DX issues which this PR tries to solve.
Tip
The DOCUMENTATION.md diff explains how this new setup works.
Issues
There is just one
connection
config for both errors and redirects.This meant we had to create our own custom repository to use a seperate connection for redirects. This PR adds seperate
redirect_connection
anderror_connection
config values.The default sqlite database is called
errors.sqlite
It took a little while until I realized that eloquent redirects are stored here as well. This PR renames the database to
redirect.sqlite
.The
README.md
is outdatedThe repository classes mentioned do not exist anymore. This PR removes outdated and duplicate documentation in the
README.md
and adds updated documentation to theDOCUMENTATION.md
.The connection name
redirect
was unclear at firstRenaming it to
redirect-sqlite
clarified this.The migration name
create_redirect_tables
implies that it contains all redirect tablesYou need to open the migration files to figure out the difference between:
create_redirect_tables
create_eloquent_redirect_table
This PR renames them to:
create_redirect_error_tables
create_redirect_redirects_table
The different migrations contain clearer tags. This makes it easier for developers to publish just the migration(s) they need.
Considerations
connection
config.connection
currently overrideserror_connection
andredirect_connection
. In my opinion, it would be better to create a new major version and dropconnection
support.errors.sqlite
toredirect.sqlite
rename. This causes problems as the boot method in the service provider runs before the update scripts. This is the reason why this logic has been put in the boot method.