Summary

Fix looking up Subversion repositories by UUID.

Review Request #8226 — Created June 9, 2016 and submitted June 22, 2016, 2:04 p.m.

Information

Owner

chipx86

Repository

RBTools

Branch

release-0.7.x

Bugs

Depends On

Commit

5d53f25...

Reviewers

Groups

rbtools

People

Description

In RBTools 0.7.6, we introduced new helpers for iterating over pages of
API list resources, and switched the SVN repository lookups to use it.
Unfortunately, switching form a list to a generator meant that we were
no longer able to loop over the requests a second time for the UUID
checks.

We now store a cached list of the repositories we iterate through, so
that we can loop over them for the UUID step (if needed). We do this
instead of immediately converting the generator to a list in order to
reduce lookups against the API.

Unit tests were added to ensure the conditions all work and won't
regress again.

Testing Done

Tested against a list of repositories and verified that both loops
were able to operate on the entire list of repositories from the API.

Issues

Description	From	Last Updated
Why is supports_parent_diffs being ripped out?	brennie	June 9, 2016, 6:32 p.m.
"match a repository" instead of "match a match"?	gmyers	June 14, 2016, 4:58 p.m.
Why not just do repositories = list(server.get_repositories(tool='Subversion').all_items) ?	david	June 14, 2016, 6:09 p.m.

Tool: Pyflakes
Processed Files:
    rbtools/api/tests.py
    rbtools/clients/svn.py
    rbtools/clients/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/api/tests.py
    rbtools/clients/svn.py
    rbtools/clients/tests.py

rbtools/clients/svn.py (Diff revision 1)

The issue has been dropped. Show all issues

Why is supports_parent_diffs being ripped out?

chipx86

June 9, 2016, 12:14 p.m.

We never pass a value for this, and it's already False by default in the parent constructor.

Ship it!

```
Patch worked for me, thanks!
```

Ship it!

rbtools/clients/svn.py (Diff revision 1)
The issue has been resolved. Show all issues
```
"match a repository" instead of "match a match"?
```

Change Summary:

Fixed a comment to not be silly.

Commit:

f821ce99bdea62e2891a367d2bde0ae4d1e1b617

fff4dd9c4273764b50390ee696fbdb1451c98c8a

Diff:

Revision 2 (+268 -28)

Show changes

	rbtools/api/tests.py
	rbtools/clients/svn.py
	rbtools/clients/tests.py

Tool: Pyflakes
Processed Files:
    rbtools/api/tests.py
    rbtools/clients/svn.py
    rbtools/clients/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/api/tests.py
    rbtools/clients/svn.py
    rbtools/clients/tests.py

rbtools/clients/svn.py (Diff revision 2)

The issue has been dropped. Show all issues

Why not just do repositories = list(server.get_repositories(tool='Subversion').all_items) ?

chipx86

June 14, 2016, 5:31 p.m.

I can elaborate in the comment, but say you have 5 pages of Subversion repositories. If your repository isn't on page 5, you don't want to have to fetch page 5. By doing it the way I have it in code, if it's on page 1, or 2, etc., we have it and don't have to look any further.

david

June 14, 2016, 5:46 p.m.

That makes sense. Elaboration in the comment would be good.

Change Summary:

Elaborated in the comment about why we're not just fetching all results up-front as a list.

Commit:

fff4dd9c4273764b50390ee696fbdb1451c98c8a

5d53f25f2dd3fcf1df9ff2da5afa4d63a8a813f7

Diff:

Revision 3 (+273 -28)

Show changes

	rbtools/api/tests.py
	rbtools/clients/svn.py
	rbtools/clients/tests.py

Tool: Pyflakes
Processed Files:
    rbtools/api/tests.py
    rbtools/clients/svn.py
    rbtools/clients/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/api/tests.py
    rbtools/clients/svn.py
    rbtools/clients/tests.py

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-0.7.x (ad7dfc5)