Generate WebAPI resource paths with backtracking
Review Request #10117 — Created Aug. 13, 2018 and submitted
The old method of generating fake resource paths assumed that the first
model instance that matchedresource.get_queryset(...)would be
suitable in all cases, i.e., this object would have all optional
relationships that corresspond to models. For example, it assumed that
the last modified review request would have all of the following:
- A review request draft
- File attachments
- Screenshots
- Reviews
- Draft file attachments
- Draft screenshots
- Draft reviews
- Review replies
- etc.This is problematic when we want to add a new model instance (e.g., for
the in-process DVCs work which adds new models) because all these child
objects will have to be created for the last modified model instance.Now, we use a backtracking algorithm to iterate over path-model pairs so
that if a model does not have the requisite child objects we can
continue onto the next instance. This way, we only need to add objects
to the database that correspond to new resources, instead of duplicating
objects from the previous parent object or recreating them entirely.
Built the docs.
With this patch applied, the docs in /r/10118/ build correctly.
| Description | From | Last Updated |
|---|---|---|
|
Typo in the description: "fakes resource paths" -> "fake resource paths" Also in the second paragraph: "the in process DVCs … |
 chipx86
|
|
|
E501 line too long (81 > 79 characters) |
 reviewbot
|
|
|
E501 line too long (82 > 79 characters) |
reviewbot
|
|
|
F841 local variable 'inner_exc' is assigned to but never used |
reviewbot
|
|
|
F841 local variable 'e' is assigned to but never used |
reviewbot
|
|
|
Needs to be the full namespace. |
chipx86
|
|
|
Blank line between these. |
chipx86
|
|
|
Why are we looping and then returning in the first iteration? |
chipx86
|
|
|
The wording seems pretty off: "Could not path for ...". I think this is missing a word. |
chipx86
|
|
|
Can you add an assertion error message explaining what the comment explained? That way if it's actually hit, we know … |
chipx86
|
|
|
The not (not ..) is kinda weird. Can we rewrite to inverse the clauses, so we have: if (not resource.singleton … |
chipx86
|
|
|
Mind adding a comment saying why we might be hitting this from the yield? It'll help with making this more … |
chipx86
|
|
|
Same comment as above. |
chipx86
|
|
|
bool |
chipx86
|
|
|
bool |
chipx86
|
- Depends On:
-
- Commit:
93785c0f495f0d74c47c9c98b2bc95f01246ddab58cbabbb51769f1378bd0c806bf5a27cfcb74e25- Diff:
Revision 2 (+100 -25)
- Commit:
-
58cbabbb51769f1378bd0c806bf5a27cfcb74e25236e1ff759e02c6ab3993eb529918785bd9ad8d2
- Diff:
-
Revision 3 (+100 -25)
Checks run (2 succeeded)
-
-
Typo in the description: "fakes resource paths" -> "fake resource paths"
Also in the second paragraph: "the in process DVCs work" "the in-progress DVCS work"
-
-
-
-
-
Can you add an assertion error message explaining what the comment explained? That way if it's actually hit, we know why instead of having to dig into code.
-
The
not (not ..)is kinda weird. Can we rewrite to inverse the clauses, so we have:if (not resource.singleton and ...): q = resource.get_queryset(...) ... else: try: yield ... except ... -
Mind adding a comment saying why we might be hitting this from the yield? It'll help with making this more maintainable (since it is a more complicated set of logic).
-
- Change Summary:
-
Addressed Christian's issues
- Description:
-
~ The old method of generating fakes resource paths assumed that the first
~ The old method of generating fake resource paths assumed that the first
model instance that matched resource.get_queryset(...)would besuitable in all cases, i.e., this object would have all optional relationships that corresspond to models. For example, it assumed that the last modified review request would have all of the following: - A review request draft - File attachments - Screenshots - Reviews - Draft file attachments - Draft screenshots - Draft reviews - Review replies - etc. This is problematic when we want to add a new model instance (e.g., for
~ the in process DVCs work which adds new models) because all these child ~ the in-process DVCs work which adds new models) because all these child objects will have to be created for the last modified model instance. Now, we use a backtracking algorithm to iterate over path-model pairs so
that if a model does not have the requisite child objects we can continue onto the next instance. This way, we only need to add objects to the database that correspond to new resources, instead of duplicating objects from the previous parent object or recreating them entirely. - Commit:
-
236e1ff759e02c6ab3993eb529918785bd9ad8d2a508824e55213b77f743d3de3a3f3f3b3d6983a2
- Diff:
-
Revision 4 (+98 -24)