Summary

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

Review Request #12209 — Created March 24, 2022 and submitted May 30, 2022, 11:04 a.m.

Information

Owner

david

Repository

Review Board

Branch

release-5.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

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.

Commits

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

Summary

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

Issues

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 …	chipx86	March 24, 2022, 2:49 p.m.
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 …	chipx86	March 24, 2022, 2:49 p.m.

flake8 passed.

JSHint passed.

reviewboard/hostingsvcs/service.py (Diff revision 1)

The issue has been dropped. 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.

david March 24, 2022, 2:43 p.m.

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

chipx86 March 24, 2022, 2:45 p.m.

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.

reviewboard/hostingsvcs/service.py (Diff revision 1)

The issue has been resolved. 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.

Commits:

	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.	a2f260f99afaaa88c82938da04ca1dbbca21aac2
	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

Diff:

Revision 2 (+124 -52)

Show changes

	setup.py
	reviewboard/hostingsvcs/assembla.py
	reviewboard/hostingsvcs/beanstalk.py
	reviewboard/hostingsvcs/bitbucket.py
	reviewboard/hostingsvcs/bugzilla.py
	reviewboard/hostingsvcs/codebasehq.py
	reviewboard/hostingsvcs/fedorahosted.py
	reviewboard/hostingsvcs/fogbugz.py
	15 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (1e89208)