-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add support for multiple CouchDB databases, multiple namespaces, and readOnly
configurations
#7413
Add support for multiple CouchDB databases, multiple namespaces, and readOnly
configurations
#7413
Conversation
…han-one-couch-database-for-a-single-couch-server
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #7413 +/- ##
==========================================
+ Coverage 55.53% 55.55% +0.02%
==========================================
Files 668 668
Lines 26704 26723 +19
Branches 2585 2585
==========================================
+ Hits 14829 14845 +16
- Misses 11164 11165 +1
- Partials 711 713 +2
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
readOnly
configurations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, this is awesome. I haven't tried it, but I assume this works with multiple CouchDB databases as well?
I did find one issue with search, where after the search has completed, the "loading" progress bar is still spinning permanently:
Screen.Recording.2024-01-26.at.1.35.19.PM.mov
I think we can spin that off into its own issue, though. I don't have any PR-blocking comments at this time! I tested locally and everything seems to work well.
…for-a-single-couch-server
@scottbell while you're waiting for additional review from Andrew, can you make sure our backup and restore scripts are up to date with this pattern? |
@unlikelyzero good idea! I’ll take a look |
@unlikelyzero they seem like they still work, provided you're using the default db name of |
…for-a-single-couch-server
Co-authored-by: Jesse Mazzella <ozyx@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thank you. My only request is that for consistency with the default namespace we remove the logic to create the root if it doesn't exist. Users can install My Items for additional namespaces if they desire, and the omitRoot
logic might be confusing. For the intended use-case the second read-only namespace will be created by an external script.
{ | ||
url: unnnormalizedOptions.url, | ||
namespace: DEFAULT_NAMESPACE, | ||
additionalNamespaces: [LEGACY_SPACE], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chef's kiss 🤌
} | ||
|
||
// final sanity check, ensure we have all options | ||
normalizedOptions.databases.forEach((databaseConfiguration) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it
@@ -493,6 +493,7 @@ | |||
"WCAG", | |||
"stackedplot", | |||
"Andale", | |||
"unnnormalized", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm prepared to accept unnormalized at a stretch. But unnnormalized? With three n's?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦♂️
…for-a-single-couch-server
Closes #7359 #7360
Describe your changes:
Add support for multiple CouchDB databases, namespaces, and
readOnly
configurations.All Submissions:
Author Checklist
type:
label? Note: this is not necessarily the same as the original issue.Reviewer Checklist