• 
      

    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)