Skip to content

Conversation

@dmitchell
Copy link
Contributor

Builds on the other PR for using computed ObjectId

@cpennington @singingwolfboy please review or delegate

[looks like I need to rebase]

@dmitchell
Copy link
Contributor Author

The only test failure was a flakey acceptance test.

@dmitchell
Copy link
Contributor Author

As to the abstraction, I think this may be a reasonable first step. Other options are

  • instead of passing dicts to the CRUD functions, create 3 models: index, structure, definition which are lightweight data transfer objects that support only .update .insert .delete methods. However, there's no simple equivalent for get.
  • instead of get_foo, get_bar, ... methods, have a get(type, id) and get(type, query) methods. etc for update, delete,.... The downside to this is that there should be no callers to update on definitions (update_structure is purely for the i-know-what-i'm-doing-single-version-transaction interface)

@singingwolfboy
Copy link
Contributor

Seems reasonable to me. 👍

@sarina
Copy link
Contributor

sarina commented Nov 18, 2013

@dmitchell - what's the status of this PR?

@dmitchell
Copy link
Contributor Author

Ask Cale. He stated that he objects but no actionable reason

On Nov 18, 2013, at 9:27 AM, Sarina Canelake notifications@github.com wrote:

@dmitchell - what's the status of this PR?


Reply to this email directly or view it on GitHub.

@sarina
Copy link
Contributor

sarina commented Nov 18, 2013

@cpennington ?

@cpennington
Copy link
Contributor

+1. Ultimately, I think this is the wrong level to be abstracting at, but
until we have the stuff needed to so it the right way, this at least
consolidates the places we depend on a specific db.
On Nov 18, 2013 9:42 AM, "Sarina Canelake" notifications@github.com wrote:

@cpennington https://github.com/cpennington ?


Reply to this email directly or view it on GitHubhttps://github.com/edx/edx-platform/pull/1333#issuecomment-28702604
.

@dmitchell
Copy link
Contributor Author

Rebased. I agree w/ @cpennington that the abstraction is not quite right. I intended this purely as a quick and dirty first step toward separation but the real abstraction would require trying to implement another db connector.

dmitchell added a commit that referenced this pull request Nov 20, 2013
Pull all db (mongo) functions into another file to enable easier replacement
@dmitchell dmitchell merged commit df349b9 into master Nov 20, 2013
@dmitchell dmitchell deleted the dhm/separate_pymongo branch November 20, 2013 15:45
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Oct 20, 2016
…ogwood/fix-task-atomic" (openedx#1333)

This reverts commit c0332b9, reversing
changes made to 7db6319.
giovannicimolin pushed a commit to open-craft/openedx-platform that referenced this pull request Jan 15, 2019
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.

5 participants