Use registry get_defaults() to return built-in hosting services.

Review Request #12209 — Created March 24, 2022 and submitted

Information

Review Board
release-5.0.x

Reviewers

Entry points are fantastic, and they're a great way for us to allow
third parties to integrate their own plugins with Review Board.
Unfortunately, if there's a VersionConflict present in the install, no
entry points will load, even if things will otherwise work. The result
of this is that none of our built-in services will work properly.

This change moves our built-ins directly into the registry's
get_defaults() method, rather than using entry points. Any third-party
services will still be loaded from entry points afterwards.

Started with a fresh database and virtualenv. Saw that built-in services
were available in the "Add Repository" form.

Summary ID
Use registry get_defaults() to return built-in hosting services.
Entry points are fantastic, and they're a great way for us to allow third parties to integrate their own plugins with Review Board. Unfortunately, if there's a VersionConflict present in the install, no entry points will load, even if things will otherwise work. The result of this is that none of our built-in services will work properly. This change moves our built-ins directly into the registry's `get_defaults()` method, rather than using entry points. Any third-party services will still be loaded from entry points afterwards. Testing Done: Started with a fresh database and virtualenv. Saw that built-in services were available in the "Add Repository" form.
597170903e6fbda892905bc6485d03846152d808
Description From Last Updated

Most of our get_defaults() just returns a list (in fact, that's the default in Registry itself). I don't think we …

chipx86chipx86

Aside from the above, we can simplify this considerably with: return [ getattr(import_module('reviewboard.hostingsvcs.%s' % _module), _service_cls_name) for _module, _service_cls_name in …

chipx86chipx86
chipx86
  1. 
      
  2. reviewboard/hostingsvcs/service.py (Diff revision 1)
     
     
     
    Show all issues

    Most of our get_defaults() just returns a list (in fact, that's the default in Registry itself). I don't think we need to yield anything, just return a full list of the items. The caller will always iterate through every one of them anyway.

    1. The EntryPointRegistry uses yield. I'm fine either way you want to do it.

    2. Ah, so it does. I don't think it really matters either way in this case. Yielding is fine, returning is fine. The construct below could be a ( ... ) generator expression if we want to keep it yielding.

  3. reviewboard/hostingsvcs/service.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues

    Aside from the above, we can simplify this considerably with:

    return [
        getattr(import_module('reviewboard.hostingsvcs.%s' % _module),
                _service_cls_name)
        for _module, _service_cls_name in (
            ('assembla', 'Assembla'),
            ('beanstalk', 'Beanstalk'),
            ...
        )
    ]
    

    Pretty similar to how we built up entrypoints.

  4. 
      
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-5.0.x (1e89208)