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

Remove BaseDataProxy.*by_mongo_name methods #254

Closed
lafrech opened this issue Apr 25, 2020 · 2 comments · Fixed by #257
Closed

Remove BaseDataProxy.*by_mongo_name methods #254

lafrech opened this issue Apr 25, 2020 · 2 comments · Fixed by #257
Milestone

Comments

@lafrech
Copy link
Collaborator

lafrech commented Apr 25, 2020

Those are private methods only ever used internally for the _id. We could replace them with a more specific method.

@lafrech lafrech added this to the 3.0 milestone Apr 25, 2020
@lafrech
Copy link
Collaborator Author

lafrech commented Apr 26, 2020

Here's a plan:

  • Add _id getter/setter to BaseDataProxy (no specific logic, just avoid accessing private _data)
  • Use this instead of set/get_by_mongo_name from Document class
  • Remove *_by_mongo_name from BaseDataProxy

No need to catch FieldNotLoadedError because

  • the "ID field not loaded" case is not really supported (committing would fail with an UpdateError if committing a doc with ID loaded)
  • we're removing partial anyway (Remove partial load feature #256)

@lafrech
Copy link
Collaborator Author

lafrech commented Apr 26, 2020

Alternatively, the document could be be responsible for the id field <-> _id relation:

  • No getter/setter in BaseDataProxy
  • Add pk_field property to Document

Use this property to

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 a pull request may close this issue.

1 participant