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

Move DatabaseController and Schema fully to adaptive mongo collection. #909

Merged
merged 3 commits into from
Mar 8, 2016

Conversation

nlutsenko
Copy link
Contributor

This is close to the final set of changes that will get us usage of mongo all through AdaptiveCollection API set.
This one moves Schema.js, DatabaseController to it as well as improves SchemasRouter API usage a little bit.

.then(() => {
let promises = insertedFields.map(fieldName => {
const mongoType = mongoObject.result[fieldName];
return schema.validateField(className, fieldName, mongoType);
Copy link
Contributor

Choose a reason for hiding this comment

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

The reason I didn't use validateField originally is that it means you only need a single database operation to handle all the additions. Can you preserve that behaviour here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can try, but am not sure why this should be blocking, since you don't have atomicity guarantees on deletions anyhow, meaning that the entire operation is not atomic...

Can this survive without having a single write for inserts? I can add a todo... 😁

Copy link
Contributor

Choose a reason for hiding this comment

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

Yah there already a lot of failure modes but this does introduce a new failure mode where some fields are added and some are not. Although this eliminates the failure mode where one person adds a different schema in the middle and then it gets clobbered, so it's really just a trade from one failure mode to another, which is probably fine. In practise I doubt there would be many writers to the schema at the same time anyway.

@nlutsenko nlutsenko force-pushed the nlutsenko.databaseController branch from f27431c to 49eb9df Compare March 8, 2016 08:53
@nlutsenko nlutsenko self-assigned this Mar 8, 2016
@facebook-github-bot
Copy link

@nlutsenko updated the pull request.

@codecov-io
Copy link

Current coverage is 91.01%

Merging #909 into master will increase coverage by +0.09% as of 8812e43

@@            master    #909   diff @@
======================================
  Files           72      72       
  Stmts         4155    4161     +6
  Branches       861     860     -1
  Methods          0       0       
======================================
+ Hit           3778    3787     +9
  Partial         10      10       
+ Missed         367     364     -3

Review entire Coverage Diff as of 8812e43

Powered by Codecov. Updated on successful CI builds.

nlutsenko added a commit that referenced this pull request Mar 8, 2016
Move DatabaseController and Schema fully to adaptive mongo collection.
@nlutsenko nlutsenko merged commit 241cd8c into master Mar 8, 2016
@nlutsenko nlutsenko deleted the nlutsenko.databaseController branch March 8, 2016 09:11
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.

4 participants