Generate WebAPI resource paths with backtracking

Review Request #10117 — Created Aug. 13, 2018 and submitted

brennie
Review Board
release-4.0.x
10112
10118
a508824...
reviewboard

The old method of generating fake resource paths assumed that the first
model instance that matched resource.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.

  • 0
  • 0
  • 15
  • 0
  • 15
Description From Last Updated
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

brennie
Review request changed

Depends On:

+10112 - Update docs fixtures with DVCS model changes

Commit:

-93785c0f495f0d74c47c9c98b2bc95f01246ddab
+58cbabbb51769f1378bd0c806bf5a27cfcb74e25

Diff:

Revision 2 (+100 -25)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
chipx86
  1. 
      
  2. 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"

    1. Still has "DVCs". Should be "DVCS".

      Also, "in-process" should be "in-progress," right?

  3. docs/manual/_ext/webapidocs.py (Diff revision 3)
     
     

    Needs to be the full namespace.

  4. docs/manual/_ext/webapidocs.py (Diff revision 3)
     
     
     

    Blank line between these.

  5. docs/manual/_ext/webapidocs.py (Diff revision 3)
     
     
     
     

    Why are we looping and then returning in the first iteration?

  6. docs/manual/_ext/webapidocs.py (Diff revision 3)
     
     

    The wording seems pretty off: "Could not path for ...". I think this is missing a word.

  7. docs/manual/_ext/webapidocs.py (Diff revision 3)
     
     

    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.

  8. docs/manual/_ext/webapidocs.py (Diff revision 3)
     
     
     
     
     

    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 ...
    
  9. docs/manual/_ext/webapidocs.py (Diff revision 3)
     
     
     

    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).

    1. I apparently had a brain fart while I was writing this. This code does nothing.

  10. docs/manual/_ext/webapidocs.py (Diff revision 3)
     
     
     

    Same comment as above.

  11. 
      
brennie
chipx86
  1. 
      
  2. docs/manual/_ext/webapidocs.py (Diff revision 4)
     
     
  3. docs/manual/_ext/webapidocs.py (Diff revision 4)
     
     
  4. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (1625f5c)
Loading...