Add fetching of git/hg repositories

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

misery
ReviewBot
release-1.0.x
9498
d42c494...
reviewbot

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.

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

flake8

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

    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)
     
     

    Can we call this review_board_servers?

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

    Should be "Review Board"

  4. bot/reviewbot/repositories.py (Diff revision 4)
     
     

    Same here.

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

    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)
     
     

    How about "Automatically Fetch Repositories From Review Board"

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

    useable -> usable

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

    reviewboard -> Review Board server.

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

    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. In your description and testing done, "ReviewBot" -> "Review Bot"

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

    Please add an "Args" section to this docstring.

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

    This should say tasks.py rather than celery.py

  5. bot/reviewbot/repositories.py (Diff revision 5)
     
     

    Indent this one more space.

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

    Maybe switch the success/error cases here?

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

    Typo: useable -> usable

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

    Please add an "Args" section to this docstring.

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

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

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

    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)
     
     

    Can we call this fetch_repositories?

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

    url is missing.

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

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

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

    Same here re: tuples vs lists.

  6. bot/reviewbot/repositories.py (Diff revision 6)
     
     
     

    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)
     
     

    No blank line here.

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

    HTTP

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

    1. Fixed.... ssh: see above

  9. 
      
misery
misery
Review request changed

Commit:

-8a0700ecc04219df1dfc99a8d1b9edbee8885ffd
+d42c494cad5aad33d6b63f1fc9501b63a51f6053

Diff:

Revision 8 (+119 -12)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
misery
  1. 
      
  2. Happy Birthday, Review Request! :(

  3. 
      
Loading...