Add fetching of git/hg repositories

Review Request #9502 — Created Jan. 20, 2018 and submitted

Information

ReviewBot
release-1.0.x
d42c494...

Reviewers

If there are a lot of Review Bot worker it will be
very expensive to maintain a repository configuration
manually. Since a reviewboard instance already knows
the configuration it is now be possible to configure
multiple reviewboard instances and let the Review Bot
fetch git and hg repositories automatically.

If a repository has a "path" or "mirror_path" that
can be access locally or has a scheme starting with
"http" or "git" it will be added to known repository.

Added some repositories in reviewboard with path
and mirror_path. Some with non-existing local path
and mirror_path with "http" scheme.

Saw that the Review Bot finds all accessible
repositories are added correctly.

Tested with and without authentication.

Description From Last Updated

In your description and testing done, "ReviewBot" -> "Review Bot"

daviddavid

W503 line break before binary operator

reviewbotreviewbot

TODO: needs to paginate here

miserymisery

Can we call this review_board_servers?

daviddavid

Should be "Review Board"

daviddavid

Same here.

daviddavid

Let's say: If you have many workers and repositories, it may not be feasible to configure repositories by hand. You …

daviddavid

How about "Automatically Fetch Repositories From Review Board"

daviddavid

useable -> usable

daviddavid

reviewboard -> Review Board server.

daviddavid

Let's rewrite this to say: The repository path or mirror_path field must be the URL of a repository which is …

daviddavid

Please add an "Args" section to this docstring.

daviddavid

This should say tasks.py rather than celery.py

daviddavid

Indent this one more space.

daviddavid

Maybe switch the success/error cases here?

daviddavid

Typo: useable -> usable

daviddavid

Please add an "Args" section to this docstring.

daviddavid

I'd rather keep this explicitly as "git" and not introduce the case insensitivity.

daviddavid

Can be repo_type in ('hg', 'mercurial') (and probably should keep everything lower case)

daviddavid

Can we call this fetch_repositories?

brenniebrennie

url is missing.

brenniebrennie

Instead of constructing a list, you can use a tuple here.

brenniebrennie

Same here re: tuples vs lists.

brenniebrennie

There might be ssh:// URLs too? If it isn't a file path, should we parse path and check (a) it …

brenniebrennie

No blank line here.

brenniebrennie

HTTP Technically, it could also be exposed over SSH, no?

brenniebrennie
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

misery
misery
misery
misery
  1. 
      
  2. bot/reviewbot/repositories.py (Diff revision 3)
     
     
     
    Show all issues

    TODO: needs to paginate here

    1. how to do that here??

    2. RBTools 1.0 has support to do this built in. If you upgrade the dependency to 1.0, you can iterate with one of the follwoing:

      for page in root.get_repositories(...).all_pages:
          for repository in page:
              # ...
      
      for repository in root.get_repositories(...).all_items:
          # ...
      
    3. Thank you very much!

  3. 
      
misery
misery
  1. 
      
  2. Ping! Any chance to merge it?

    1. :-( contribution to review board is no fun

    2. I'm sorry. We've been very busy these last couple months.

  3. 
      
david
  1. 
      
  2. bot/reviewbot/config.py (Diff revision 4)
     
     
    Show all issues

    Can we call this review_board_servers?

  3. bot/reviewbot/repositories.py (Diff revision 4)
     
     
    Show all issues

    Should be "Review Board"

  4. bot/reviewbot/repositories.py (Diff revision 4)
     
     
    Show all issues

    Same here.

  5. docs/reviewbot/configuration.rst (Diff revision 4)
     
     
     
    Show all issues

    Let's say:

    If you have many workers and repositories, it may not be feasible to configure repositories by hand. You can also configure a list of Review Board servers to fetch all supported repositories from.

  6. docs/reviewbot/configuration.rst (Diff revision 4)
     
     
    Show all issues

    How about "Automatically Fetch Repositories From Review Board"

  7. docs/reviewbot/configuration.rst (Diff revision 4)
     
     
    Show all issues

    useable -> usable

  8. docs/reviewbot/configuration.rst (Diff revision 4)
     
     
    Show all issues

    reviewboard -> Review Board server.

  9. docs/reviewbot/configuration.rst (Diff revision 4)
     
     
     
     
     
     
     
     
     
     
    Show all issues

    Let's rewrite this to say:

    The repository path or mirror_path field must be the URL of a repository which is accessible to the Review Bot worker. If you use a local file path for your repository and the worker is not running on the same host as the Review Board server, you must also expose the repository over http and set the mirror_path.

    This should probably also be a separate paragraph from the "manually configured repositories will override"

  10. 
      
misery
david
  1. 
      
  2. Show all issues

    In your description and testing done, "ReviewBot" -> "Review Bot"

  3. bot/reviewbot/repositories.py (Diff revision 5)
     
     
    Show all issues

    Please add an "Args" section to this docstring.

  4. bot/reviewbot/repositories.py (Diff revision 5)
     
     
    Show all issues

    This should say tasks.py rather than celery.py

  5. bot/reviewbot/repositories.py (Diff revision 5)
     
     
    Show all issues

    Indent this one more space.

  6. bot/reviewbot/repositories.py (Diff revision 5)
     
     
    Show all issues

    Maybe switch the success/error cases here?

  7. bot/reviewbot/repositories.py (Diff revision 5)
     
     
    Show all issues

    Typo: useable -> usable

  8. bot/reviewbot/repositories.py (Diff revision 5)
     
     
    Show all issues

    Please add an "Args" section to this docstring.

  9. bot/reviewbot/repositories.py (Diff revision 5)
     
     
    Show all issues

    I'd rather keep this explicitly as "git" and not introduce the case insensitivity.

  10. bot/reviewbot/repositories.py (Diff revision 5)
     
     
    Show all issues

    Can be repo_type in ('hg', 'mercurial') (and probably should keep everything lower case)

  11. 
      
misery
brennie
  1. 
      
  2. bot/reviewbot/repositories.py (Diff revision 6)
     
     
    Show all issues

    Can we call this fetch_repositories?

  3. bot/reviewbot/repositories.py (Diff revision 6)
     
     
    Show all issues

    url is missing.

  4. bot/reviewbot/repositories.py (Diff revision 6)
     
     
    Show all issues

    Instead of constructing a list, you can use a tuple here.

  5. bot/reviewbot/repositories.py (Diff revision 6)
     
     
    Show all issues

    Same here re: tuples vs lists.

  6. bot/reviewbot/repositories.py (Diff revision 6)
     
     
     
    Show all issues

    There might be ssh:// URLs too?

    If it isn't a file path, should we parse path and check (a) it is a valid URL and (b) its scheme is in ('http', 'https', 'ssh', 'git', 'git+ssh') ?

    1. Yeah, it is possible. But that is another feature that should be tested well because of authentication with key/password. I prefer to create a following review request to support ssh style.

  7. bot/reviewbot/repositories.py (Diff revision 6)
     
     
    Show all issues

    No blank line here.

  8. docs/reviewbot/configuration.rst (Diff revision 6)
     
     
    Show all issues

    HTTP

    Technically, it could also be exposed over SSH, no?

    1. Fixed.... ssh: see above

  9. 
      
misery
misery
misery
  1. 
      
  2. Happy Birthday, Review Request! :(

  3. 
      
david
  1. Making some small changes to the docs and landing. Sorry for the delay on this.

  2. 
      
misery
Review request changed
Status:
Completed
Change Summary:
Pushed to master (67d4f05)