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.

Sign up for GitHub

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

Jump to bottom

Add feature to disable using default tenant to store multi tenanted docs #1450

Merged
merged 1 commit into from
May 25, 2020
Merged

Add feature to disable using default tenant to store multi tenanted docs #1450

merged 1 commit into from
May 25, 2020

Conversation

mysticmind
Copy link
Member

  • Add feature to disable using default tenant to store multi tenanted docs
  • Add unit tests
  • Add docs

Fixes GH-1449

@mysticmind
Add feature to disable using default tenant to store multi tenanted docs
2148798 
- Add feature to disable using default tenant to store multi tenanted
docs
- Add unit tests
- add docs
@tonykaralis
Copy link
Contributor

This is exactly what I was looking for. Thanks @mysticmind

oskardudycz
oskardudycz approved these changes Apr 7, 2020
View reviewed changes
Copy link
Collaborator

@oskardudycz oskardudycz left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@mysticmind
Copy link
Member Author

@jeremydmiller @jokokko please review the PR.

jokokko
jokokko requested changes Apr 14, 2020
View reviewed changes
Copy link
Collaborator

@jokokko jokokko left a comment

Choose a reason for hiding this comment

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

Was thinking, if IDocumentStore.BulkInsert needs to be covered by this setting too. The other thing I noticed is the XML doc wording "when using multi-tenanted documents", but there are no explicit checks whether multitenancy is being used. Other than that, everything looks good to go.

Overall, this feature looks a bit niche, which lead me to think if we should try to group new settings under store options (i.e. under another type) or have some advanced section (well, we also have the AdvancedOptions so that StoreOptions won't become overwheling to a new user.

@jeremydmiller jeremydmiller mentioned this pull request May 25, 2020
Closed
@jeremydmiller jeremydmiller added this to the 3.12.0 milestone May 25, 2020
@jeremydmiller jeremydmiller merged commit 32a0fd2 into JasperFx:master May 25, 2020
@jeremydmiller
Copy link
Member

I'm pulling this in now, but I'm going to deal with the bulk insert like @jokokko suggested

@jeremydmiller
Copy link
Member

I actually think it's fine the way it is now. It's just adding a new assertion on starting up a document session

@mysticmind mysticmind deleted the disable_default_tenant_usage_feature branch June 1, 2020 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jokokko jokokko jokokko requested changes

@oskardudycz oskardudycz oskardudycz approved these changes

@jeremydmiller jeremydmiller Awaiting requested review from jeremydmiller

Assignees
No one assigned
Labels
enhancement multi-tenancy
Projects
None yet
Milestone
3.12.0
Development

Successfully merging this pull request may close these issues.

Disable default tenancy via store options
5 participants
@mysticmind @tonykaralis @jeremydmiller @jokokko @oskardudycz