Summary

Add integration model and manager for third-party services framework

Review Request #6918 — Created Feb. 7, 2015 and discarded April 28, 2016, 2:21 a.m.

Information

Owner

xuanyi

Repository

Review Board

Branch

master

Bugs

Depends On

Blocks

7221, 7105, 7136, 7106, 7034, 7033

Reviewers

Groups

reviewboard, students

People

Description

This provides the integration base class, database model and manager for extensions to implement and register integrations.
Compared to the extension framework, these integration model will provide a way for users, such as local site administrator, to manage their own individiual integration. Third-party services will need to subclass the integration base class to provide the initialize and shutdown method for their services. 
With the database model, multiple instances of the integration's configuration could be supported, and thus providing multiple configurations of the integration for a single web server. 
All these configured integration will then be managed through the integration manager.

Testing Done

Added unit test for the integrations.

Test manager on creating, deleting, toggling of configured integration.
Test manager on handling of invalid configured integration.

Mannual testing with a dummy integration.

Unit tests pass.

Issues

Description	From	Last Updated
'django_reset' imported but unused	reviewbot	Feb. 7, 2015, 9:17 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Feb. 7, 2015, 9:17 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	Feb. 7, 2015, 9:17 p.m.
'PIPELINE_JS' imported but unused	reviewbot	Feb. 7, 2015, 9:17 p.m.
'django_reset' imported but unused	reviewbot	Feb. 14, 2015, 8:31 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Feb. 14, 2015, 8:31 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	Feb. 14, 2015, 8:31 p.m.
'PIPELINE_JS' imported but unused	reviewbot	Feb. 14, 2015, 8:31 p.m.
'django_reset' imported but unused	reviewbot	Feb. 14, 2015, 8:37 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Feb. 14, 2015, 8:37 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	Feb. 14, 2015, 8:37 p.m.
'PIPELINE_JS' imported but unused	reviewbot	Feb. 14, 2015, 8:37 p.m.
"Allows extensions to register and unregister service integrations."	chipx86	March 13, 2015, 7:50 a.m.
Two blank lines are needed here.	chipx86	March 13, 2015, 7:50 a.m.
Must be inserted in alphabetical order.	chipx86	March 13, 2015, 7:50 a.m.
When the imports can fit on one line, they should be on one line without parens. However, instead of importing …	chipx86	March 13, 2015, 7:50 a.m.
"registration"	chipx86	March 13, 2015, 7:50 a.m.
"unregistration"	chipx86	March 13, 2015, 7:50 a.m.
We don't want to introduce any new fixtures into the codebase. Instead, create the objects as you need them. See …	chipx86	March 13, 2015, 7:50 a.m.
Each of these should be documented, using this form: #: Single line comment myvar1 = ... #: Single-line summary #: …	chipx86	March 13, 2015, 7:50 a.m.
Blank line between these.	chipx86	March 13, 2015, 7:50 a.m.
Why is there a separate initialize method? Why can't subclass initialization be done in __init__?	brennie	March 21, 2015, 9:49 a.m.
"URL"	chipx86	March 13, 2015, 7:50 a.m.
Two blank lines between these.	chipx86	March 13, 2015, 6:22 a.m.
No blank line here.	brennie	March 13, 2015, 7:53 a.m.
"Return" instead of "Get".	chipx86	March 13, 2015, 7:50 a.m.
"Returns" instead of "Retrieves", "ID" instead of "id", and a period is needed.	chipx86	March 13, 2015, 7:50 a.m.
No blank line here.	chipx86	March 13, 2015, 7:51 a.m.
Blank line after class docstrings.	chipx86	March 13, 2015, 7:51 a.m.
Private functions should always come after all the public functions.	chipx86	March 13, 2015, 7:51 a.m.
This can be combined into one statement.	chipx86	March 13, 2015, 7:51 a.m.
Should use pk instead of id for model instances.	chipx86	March 13, 2015, 7:51 a.m.
How about just: "This configuration is already registered."	chipx86	March 13, 2015, 7:51 a.m.
"ID"	chipx86	March 13, 2015, 7:52 a.m.
This needs to not break loading the rest of the integrations. If an extension has been disabled, the ConfiguredIntegrations will …	chipx86	March 13, 2015, 7:52 a.m.
Missing a period.	chipx86	March 13, 2015, 7:52 a.m.
This would be better as: try: del self._config_instances[config_instance.pl] except KeyError: raise ... Also, note pk instead of id for model …	chipx86	March 13, 2015, 7:52 a.m.
Missing a trailing period.	chipx86	March 13, 2015, 7:52 a.m.
Trailing period.	chipx86	March 13, 2015, 7:52 a.m.
Change the get to a filter, to save an SQL query. This way, we don't have to fetch the entire …	chipx86	March 13, 2015, 7:52 a.m.
Trailing period.	chipx86	March 13, 2015, 7:52 a.m.
Trailing period.	chipx86	March 13, 2015, 7:52 a.m.
Trailing period.	chipx86	March 13, 2015, 7:52 a.m.
Trailing period.	chipx86	March 13, 2015, 7:52 a.m.
Trailing period.	chipx86	March 13, 2015, 7:52 a.m.
Trailing period.	chipx86	March 13, 2015, 7:52 a.m.
Should use six.itervalues(...) instead of calling values() on the dictionary.	chipx86	March 13, 2015, 7:52 a.m.
Trailing period.	chipx86	March 13, 2015, 7:52 a.m.
Needs a docstring.	chipx86	March 13, 2015, 7:52 a.m.
Must be in alphabetical order.	chipx86	March 13, 2015, 7:52 a.m.
Needs a docstring.	chipx86	March 13, 2015, 7:52 a.m.
Needs a docstring. This should use @cached_property instead of doing the hasattr trick. Search the codebase for the proper import …	chipx86	March 13, 2015, 7:53 a.m.
"has" access.	chipx86	March 13, 2015, 7:53 a.m.
Must be a single line. How about: "Return whether the user can modify this configuration."	chipx86	March 13, 2015, 7:53 a.m.
This needs to return something.	chipx86	March 21, 2015, 9:49 a.m.
Must be in alphabetical order.	chipx86	March 13, 2015, 7:53 a.m.
Trailing period.	chipx86	March 13, 2015, 7:53 a.m.
Multi-line unit test docstrings must be in this form: """First line second line. """	chipx86	March 13, 2015, 7:53 a.m.
No blank line here.	chipx86	March 13, 2015, 7:53 a.m.
'django_reset' imported but unused	reviewbot	March 2, 2015, 8:09 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	March 2, 2015, 8:09 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	March 2, 2015, 8:09 p.m.
'PIPELINE_JS' imported but unused	reviewbot	March 5, 2015, 12:20 a.m.
Col: 13 E123 closing bracket does not match indentation of opening bracket's line	reviewbot	March 13, 2015, 8:05 a.m.
'django_reset' imported but unused	reviewbot	March 13, 2015, 8:05 a.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	March 13, 2015, 8:05 a.m.
'PIPELINE_JS' imported but unused	reviewbot	March 13, 2015, 8:05 a.m.
'PIPELINE_CSS' imported but unused	reviewbot	March 13, 2015, 8:05 a.m.
'django_reset' imported but unused	reviewbot	March 13, 2015, 8:05 a.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	March 13, 2015, 8:05 a.m.
'PIPELINE_JS' imported but unused	reviewbot	March 13, 2015, 8:05 a.m.
'PIPELINE_CSS' imported but unused	reviewbot	March 13, 2015, 8:05 a.m.
'reviewboard' imported but unused	reviewbot	March 21, 2015, 8:46 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	March 21, 2015, 8:46 p.m.
Col: 5 E303 too many blank lines (2)	reviewbot	March 21, 2015, 8:46 p.m.
'django_reset' imported but unused	reviewbot	March 21, 2015, 8:46 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	March 21, 2015, 8:46 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	March 21, 2015, 8:46 p.m.
'PIPELINE_JS' imported but unused	reviewbot	March 21, 2015, 8:46 p.m.
'reviewboard' imported but unused	reviewbot	March 21, 2015, 8:46 p.m.
'django_reset' imported but unused	reviewbot	March 21, 2015, 8:46 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	March 21, 2015, 8:46 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	March 21, 2015, 8:46 p.m.
'PIPELINE_JS' imported but unused	reviewbot	March 21, 2015, 8:46 p.m.
'reviewboard' imported but unused	reviewbot	March 29, 2015, 2:32 a.m.
This shouldn't be set here. Rather, Integrations should accept an extension in the constructor, but only if that specific integration …	chipx86	April 25, 2015, 9:52 p.m.
Each comment before an attribute should be in this form: #: Summary #: #: ... Note the #:. That will …	chipx86	April 5, 2015, 12:55 a.m.
Should be allow_local_sites.	chipx86	April 5, 2015, 12:55 a.m.
How about config_form?	chipx86	April 5, 2015, 12:55 a.m.
Instead of using join, we should just use a format string.	chipx86	April 5, 2015, 12:56 a.m.
"integration"	chipx86	April 5, 2015, 12:56 a.m.
"toggled"	chipx86	April 5, 2015, 12:56 a.m.
"Subclasses" ".. to shut down the integration and ..."	chipx86	April 5, 2015, 12:56 a.m.
"... to be toggled ..."	chipx86	April 5, 2015, 12:56 a.m.
No need for the None parameter. That's the default.	chipx86	April 5, 2015, 12:56 a.m.
I don't think we should have a load method, and we certainly shouldn't call it when initializing Review Board. It …	chipx86	April 25, 2015, 9:52 p.m.
Combine these conditionals.	chipx86	April 5, 2015, 12:56 a.m.
This is pretty redundant. We should have one or the other.	chipx86	April 5, 2015, 12:56 a.m.
"integration"	chipx86	April 5, 2015, 12:56 a.m.
This can be one statement.	chipx86	April 14, 2015, 6:33 p.m.
Swap these (alphabetical order).	chipx86	April 5, 2015, 12:56 a.m.
""" on the next line.	chipx86	April 5, 2015, 12:56 a.m.
Here too. Same below.	chipx86	April 5, 2015, 12:56 a.m.
'django_reset' imported but unused	reviewbot	March 29, 2015, 2:32 a.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	March 29, 2015, 2:32 a.m.
'PIPELINE_JS' imported but unused	reviewbot	March 29, 2015, 2:32 a.m.
'PIPELINE_CSS' imported but unused	reviewbot	March 29, 2015, 2:32 a.m.
"Create"	chipx86	April 5, 2015, 12:56 a.m.
'reviewboard' imported but unused	reviewbot	April 25, 2015, 9:53 p.m.
'django_reset' imported but unused	reviewbot	April 25, 2015, 9:53 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	April 25, 2015, 9:53 p.m.
'PIPELINE_JS' imported but unused	reviewbot	April 25, 2015, 9:53 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	April 25, 2015, 9:53 p.m.
'get_integration_manager' imported but unused	reviewbot	April 25, 2015, 9:59 p.m.
'reviewboard' imported but unused	reviewbot	April 25, 2015, 9:59 p.m.
'django_reset' imported but unused	reviewbot	April 25, 2015, 9:59 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	April 25, 2015, 9:59 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	April 25, 2015, 9:59 p.m.
'PIPELINE_JS' imported but unused	reviewbot	April 25, 2015, 9:59 p.m.
'django_reset' imported but unused	reviewbot	April 25, 2015, 10:03 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	April 25, 2015, 10:03 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	April 25, 2015, 10:03 p.m.
'PIPELINE_JS' imported but unused	reviewbot	April 25, 2015, 10:03 p.m.
Imports should come in alphabetical order.	david	May 19, 2015, 5:39 p.m.
Should be in the imperative mood ("Wrap a callback ...")	david	May 19, 2015, 5:39 p.m.
exc_info should be True, rather than 1 (we have 1 a bunch of other places, but I'm slowly cleaning those …	david	May 19, 2015, 5:39 p.m.
This should use the same terminology as you changed above ("registration" rather than "initializing")	david	May 19, 2015, 5:39 p.m.
This should use the same terminology as you changed above ("registration" rather than "initializing")	david	May 19, 2015, 5:39 p.m.
Should be in the imperative mood ("Shut down...")	david	May 19, 2015, 5:39 p.m.
Should be in the imperative mood ("Return the...")	david	May 19, 2015, 5:39 p.m.
Should be in the imperative mood ("Return the...")	david	May 19, 2015, 5:39 p.m.
Should be in the imperative mood ("Return the...")	david	May 19, 2015, 5:39 p.m.
The logging methods will do a format operation internally, so you can pass in config_id as a parameter rather than …	david	May 19, 2015, 5:39 p.m.
Should be in the imperative mood ("Return all...")	david	May 19, 2015, 5:39 p.m.
Should be in the imperative mood ("Return the...")	david	May 19, 2015, 5:39 p.m.
Should be in the imperative mood ("Return the...")	david	May 19, 2015, 5:39 p.m.
Should be in the imperative mood ("Return...")	david	May 19, 2015, 5:39 p.m.
Should be in the imperative mood ("Return...")	david	May 19, 2015, 5:39 p.m.
'django_reset' imported but unused	reviewbot	May 5, 2015, 8:01 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	May 5, 2015, 8:01 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	May 5, 2015, 8:01 p.m.
'PIPELINE_JS' imported but unused	reviewbot	May 5, 2015, 8:01 p.m.

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/integrations/models.py
    reviewboard/integrations/integration.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/manager.py
    reviewboard/settings.py

Ignored Files:
    reviewboard/integrations/tests.py
    reviewboard/integrations/__init__.py



Tool: Pyflakes
Processed Files:
    reviewboard/integrations/models.py
    reviewboard/integrations/integration.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/manager.py
    reviewboard/settings.py

Ignored Files:
    reviewboard/integrations/tests.py
    reviewboard/integrations/__init__.py

reviewboard/settings.py (Diff revision 1)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 1)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 1)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 1)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```

Description:: This provides the integration base class, database model and manager for extensions to implement and register integrations.

Initial: February 8, 2015

Added IntegrationHook in hooks.py

Added integration class to be use as the base class for all integration

Added manager class to manage all integrations

Added ConfiguredIntegration model that stores the configuration of an integration
~
Register integration in settings.py
~
Registered integration in settings.py

Summary:

[WIP] Add integration model and manager for third-party services framework

Add integration model and manager for third-party services framework

Description:

		This provides the integration base class, database model and manager for extensions to implement and register integrations.

		Initial: February 8, 2015

		Added IntegrationHook in hooks.py
		Added integration class to be use as the base class for all integration
		Added manager class to manage all integrations
		Added ConfiguredIntegration model that stores the configuration of an integration
		Registered integration in settings.py
	+
	+	Update: February 15, 2015
	+
	+	Added unit tests for IntegrationManager
	+	Added unit tests to extensions.test for IntegrationHook
	+	Added fixtures

Testing Done:

~		Manual testing with a dummy integration
	~	Manual testing with a dummy integration.
	+
	+	Unit tests pass.

Commit:

1299320179c17866161267d203cb93cd21a58776

4d035decd1e3a58d078af14f830c0ba759fe9e73

Diff:

Revision 2 (+428 -1)

Show changes

	reviewboard/settings.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py
	reviewboard/integrations/__init__.py
	reviewboard/integrations/integration.py
	reviewboard/integrations/manager.py
	reviewboard/integrations/models.py
	reviewboard/integrations/tests.py
	1 more

Tool: Pyflakes
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/fixtures/test_configs.json
    reviewboard/integrations/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/fixtures/test_configs.json
    reviewboard/integrations/__init__.py

reviewboard/settings.py (Diff revision 2)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 2)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 2)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 2)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```

Commit:

4d035decd1e3a58d078af14f830c0ba759fe9e73

d52d21a7e6ef28405240d07e4b1c722b5abed591

Diff:

Revision 3 (+428 -1)

Show changes

	reviewboard/settings.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py
	reviewboard/integrations/__init__.py
	reviewboard/integrations/integration.py
	reviewboard/integrations/manager.py
	reviewboard/integrations/models.py
	reviewboard/integrations/tests.py
	1 more

Tool: Pyflakes
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/fixtures/test_configs.json
    reviewboard/integrations/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/fixtures/test_configs.json
    reviewboard/integrations/__init__.py

reviewboard/settings.py (Diff revision 3)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 3)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 3)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 3)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```

Commit:

d52d21a7e6ef28405240d07e4b1c722b5abed591

791665e60fb1f6ef70731ce9fed0cb53b3b6f36f

Diff:

Revision 4 (+427 -1)

Show changes

	reviewboard/settings.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py
	reviewboard/integrations/__init__.py
	reviewboard/integrations/integration.py
	reviewboard/integrations/manager.py
	reviewboard/integrations/models.py
	reviewboard/integrations/tests.py
	1 more

Tool: Pyflakes
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/fixtures/test_configs.json
    reviewboard/integrations/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/fixtures/test_configs.json
    reviewboard/integrations/__init__.py

reviewboard/settings.py (Diff revision 4)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 4)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 4)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 4)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```

reviewboard/extensions/hooks.py (Diff revision 4)
The issue has been resolved. Show all issues
```
"Allows extensions to register and unregister service integrations."
```
reviewboard/extensions/hooks.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Two blank lines are needed here.
```
reviewboard/extensions/tests.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Must be inserted in alphabetical order.
```

reviewboard/extensions/tests.py (Diff revision 4)

The issue has been resolved. Show all issues

When the imports can fit on one line, they should be on one line without parens.
However, instead of importing _integrations, we should import a function that can return an integration with the given ID.

reviewboard/extensions/tests.py (Diff revision 4)
The issue has been resolved. Show all issues
```
"registration"
```
reviewboard/extensions/tests.py (Diff revision 4)
The issue has been resolved. Show all issues
```
"unregistration"
```

reviewboard/integrations/fixtures/test_configs.json (Diff revision 4)

The issue has been resolved. Show all issues

We don't want to introduce any new fixtures into the codebase. Instead, create the objects as you need them.

See reviewboard/testing/testcase.py.

reviewboard/integrations/integration.py (Diff revision 4)

The issue has been resolved. Show all issues

Each of these should be documented, using this form:

#: Single line comment
myvar1 = ...

#: Single-line summary
#:
#: Description that spans
#: multiple lines.
myvar2 = ...

reviewboard/integrations/integration.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Blank line between these.
```
reviewboard/integrations/integration.py (Diff revision 4)
The issue has been resolved. Show all issues
```
"URL"
```
reviewboard/integrations/integration.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Two blank lines between these.
```
reviewboard/integrations/integration.py (Diff revision 4)
The issue has been resolved. Show all issues
```
"Return" instead of "Get".
```
reviewboard/integrations/integration.py (Diff revision 4)
The issue has been resolved. Show all issues
```
"Returns" instead of "Retrieves", "ID" instead of "id", and a period is needed.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
No blank line here.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Blank line after class docstrings.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Private functions should always come after all the public functions.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
This can be combined into one statement.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Should use pk instead of id for model instances.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
How about just: "This configuration is already registered."
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
"ID"
```

reviewboard/integrations/manager.py (Diff revision 4)

The issue has been resolved. Show all issues

This needs to not break loading the rest of the integrations. If an extension has been disabled, the ConfiguredIntegrations will still be in the database, which is completely valid.

What I'd suggest is not erroring out if the integration is not registered. Instead, silently skip it.

reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Missing a period.
```

reviewboard/integrations/manager.py (Diff revision 4)

The issue has been resolved. Show all issues

This would be better as:

try:
    del self._config_instances[config_instance.pl]
except KeyError:
    raise ...

Also, note pk instead of id for model instances.

I'd change the test like I suggested above.

reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Missing a trailing period.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Trailing period.
```

reviewboard/integrations/manager.py (Diff revision 4)

The issue has been resolved. Show all issues

Change the get to a filter, to save an SQL query. This way, we don't have to fetch the entire object before deleting it.

reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Trailing period.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Trailing period.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Trailing period.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Trailing period.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Trailing period.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Trailing period.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Should use six.itervalues(...) instead of calling values() on the dictionary.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Trailing period.
```
reviewboard/integrations/manager.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Needs a docstring.
```
reviewboard/integrations/models.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Must be in alphabetical order.
```
reviewboard/integrations/models.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Needs a docstring.
```

reviewboard/integrations/models.py (Diff revision 4)

The issue has been resolved. Show all issues

Needs a docstring.
This should use @cached_property instead of doing the hasattr trick. Search the codebase for the proper import statement.

reviewboard/integrations/models.py (Diff revision 4)
The issue has been resolved. Show all issues
```
"has" access.
```

reviewboard/integrations/models.py (Diff revision 4)

The issue has been resolved. Show all issues

Must be a single line. How about:

"Return whether the user can modify this configuration."

reviewboard/integrations/models.py (Diff revision 4)

The issue has been resolved. Show all issues

This needs to return something.

xuanyi March 13, 2015, 8:38 a.m.

Hi Christian,

Thank you for putting in so much effort in reviewing my code. I really appreciate it.

With regards to this issue, I am still confuse about how I should go about setting the permission. For the other models which dealt with LocalSite, the is_mutable_by method will be returning the permission base on _VALID_LOCAL_SITE_PERMISSIONS in backend.py. However, since integration may not solely deal with LocalSite, I was wondering whether I could make use of this to achieve the same purpose. Also, since the views depends on these permission settings, I was wondering how it will affect the admin access control in managing the integrations.

reviewboard/integrations/tests.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Must be in alphabetical order.
```
reviewboard/integrations/tests.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Trailing period.
```

reviewboard/integrations/tests.py (Diff revision 4)

The issue has been resolved. Show all issues

Multi-line unit test docstrings must be in this form:

"""First line second line. """

reviewboard/integrations/tests.py (Diff revision 4)
The issue has been resolved. Show all issues
```
No blank line here.
```

reviewboard/integrations/integration.py (Diff revision 4)

The issue has been resolved. Show all issues

Why is there a separate initialize method? Why can't subclass initialization be done in __init__?

xuanyi March 13, 2015, 8:38 a.m.

Hi Barret,

Thank you so much for reviewing my code!

Regarding this issue, I have abstract out the intialize component because of the following two reasons:
1) In the case of toggling, the integration will have to be re-initialize after shutting down. With the initialize method, the toggle method could call it to re-initialize the integration.
2) The alternative will be to recreate the object after it has been shutdown. However, as each configured integration is tied to an integration instance, I am wondering whether it will be easier to reuse that instance to restart the service instead of recreating a new instance.

Sorry about the confusion I have over this part. I will make appropriate change to it if it is not right.

brennie March 13, 2015, 9:12 a.m.

OK! This seems like a good approach to me. However, this information would be really good for the docstring so that others know that initialize/shutdown is for the toggling of the instances after they have been created.

reviewboard/integrations/integration.py (Diff revision 4)
The issue has been resolved. Show all issues
```
No blank line here.
```

Commit:

791665e60fb1f6ef70731ce9fed0cb53b3b6f36f

bb05d3e087daed1eb9c8b08905da7c713d562a51

Diff:

Revision 5 (+461 -2)

Show changes

	reviewboard/settings.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py
	reviewboard/integrations/__init__.py
	reviewboard/integrations/integration.py
	reviewboard/integrations/manager.py
	reviewboard/integrations/models.py
	reviewboard/integrations/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py

reviewboard/integrations/tests.py (Diff revision 5)
The issue has been resolved. Show all issues
```
Col: 13
 E123 closing bracket does not match indentation of opening bracket's line
```
reviewboard/settings.py (Diff revision 5)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 5)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 5)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```
reviewboard/settings.py (Diff revision 5)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```

Commit:

bb05d3e087daed1eb9c8b08905da7c713d562a51

8955efcaad9674e4b65da6e164cc92ecfad9c963

Diff:

Revision 6 (+460 -2)

Show changes

	reviewboard/settings.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py
	reviewboard/integrations/__init__.py
	reviewboard/integrations/integration.py
	reviewboard/integrations/manager.py
	reviewboard/integrations/models.py
	reviewboard/integrations/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py

reviewboard/settings.py (Diff revision 6)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 6)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 6)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```
reviewboard/settings.py (Diff revision 6)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```

Commit:

8955efcaad9674e4b65da6e164cc92ecfad9c963

2d76fd290f9026538955d85ff2702561574c32ec

Diff:

Revision 7 (+478 -2)

Show changes

	reviewboard/__init__.py
	reviewboard/settings.py
	reviewboard/accounts/backends.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py
	reviewboard/integrations/__init__.py
	reviewboard/integrations/integration.py
	reviewboard/integrations/manager.py
	2 more

Tool: Pyflakes
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/__init__.py
    reviewboard/accounts/backends.py
    reviewboard/integrations/models.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/__init__.py
    reviewboard/accounts/backends.py
    reviewboard/integrations/models.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py

reviewboard/__init__.py (Diff revision 7)
The issue has been dropped. Show all issues
```
 'reviewboard' imported but unused
```
reviewboard/integrations/manager.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
reviewboard/integrations/manager.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Col: 5
 E303 too many blank lines (2)
```
reviewboard/settings.py (Diff revision 7)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 7)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 7)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 7)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```

Commit:

2d76fd290f9026538955d85ff2702561574c32ec

1f5a611c3101001debe91b0e3063613bd76416a3

Diff:

Revision 8 (+478 -2)

Show changes

	reviewboard/__init__.py
	reviewboard/settings.py
	reviewboard/accounts/backends.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py
	reviewboard/integrations/__init__.py
	reviewboard/integrations/integration.py
	reviewboard/integrations/manager.py
	2 more

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/__init__.py
    reviewboard/accounts/backends.py
    reviewboard/integrations/models.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py



Tool: Pyflakes
Processed Files:
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/__init__.py
    reviewboard/accounts/backends.py
    reviewboard/integrations/models.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py

reviewboard/__init__.py (Diff revision 8)
The issue has been dropped. Show all issues
```
 'reviewboard' imported but unused
```
reviewboard/settings.py (Diff revision 8)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 8)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 8)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 8)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```

Description:

		This provides the integration base class, database model and manager for extensions to implement and register integrations.

		Initial: February 8, 2015

		Added IntegrationHook in hooks.py
		Added integration class to be use as the base class for all integration
		Added manager class to manage all integrations
		Added ConfiguredIntegration model that stores the configuration of an integration
		Registered integration in settings.py

		Update: February 15, 2015

		Added unit tests for IntegrationManager
		Added unit tests to extensions.test for IntegrationHook
		Added fixtures
	+
	+	Update: March 29, 2015
	+
	+	Implemented common test method in testcase
	+	Modified permission for models

Commit:

1f5a611c3101001debe91b0e3063613bd76416a3

709a7e7548cddf0a611b3e01103c693fd118c992

Diff:

Revision 9 (+507 -2)

Show changes

	reviewboard/__init__.py
	reviewboard/settings.py
	reviewboard/accounts/backends.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py
	reviewboard/integrations/__init__.py
	reviewboard/integrations/integration.py
	reviewboard/integrations/manager.py
	3 more

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/testing/testcase.py
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/__init__.py
    reviewboard/accounts/backends.py
    reviewboard/integrations/models.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py



Tool: Pyflakes
Processed Files:
    reviewboard/testing/testcase.py
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/__init__.py
    reviewboard/accounts/backends.py
    reviewboard/integrations/models.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py

reviewboard/__init__.py (Diff revision 9)
The issue has been dropped. Show all issues
```
 'reviewboard' imported but unused
```
reviewboard/settings.py (Diff revision 9)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 9)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 9)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```
reviewboard/settings.py (Diff revision 9)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```

General note: The Description is the wrong place for the updates. Those belong in the draft banner at the top when posting a new change. The Description should be a commit-ready, full description of this change, useful for anyone to understand what this commit provides without knowing the rest of the work going on.

reviewboard/extensions/hooks.py (Diff revision 9)

The issue has been resolved. Show all issues

This shouldn't be set here. Rather, Integrations should accept an extension in the constructor, but only if that specific integration needs to.

xuanyi April 5, 2015, 12:55 a.m.

I was making use of the extension's staticfiles finder to locate files(e.g. icon image) for the integration, which requires the "extension_id" to locate the file properly. However, the only place which the integration could know about the extension is through the initilization of the hook or in the initialize method of the extension. What will be a good way to resolve this dependency?

reviewboard/integrations/integration.py (Diff revision 9)

The issue has been resolved. Show all issues

Each comment before an attribute should be in this form:

#: Summary
#:
#: ...


Note the #:. That will let Sphinx (our docs generator) use the comments as documentation.

reviewboard/integrations/integration.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Should be allow_local_sites.
```
reviewboard/integrations/integration.py (Diff revision 9)
The issue has been resolved. Show all issues
```
How about config_form?
```
reviewboard/integrations/integration.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Instead of using join, we should just use a format string.
```
reviewboard/integrations/integration.py (Diff revision 9)
The issue has been resolved. Show all issues
```
"integration"
```
reviewboard/integrations/integration.py (Diff revision 9)
The issue has been resolved. Show all issues
```
"toggled"
```
reviewboard/integrations/integration.py (Diff revision 9)
The issue has been resolved. Show all issues
```
"Subclasses"
".. to shut down the integration and ..."
```
reviewboard/integrations/integration.py (Diff revision 9)
The issue has been resolved. Show all issues
```
"... to be toggled ..."
```
reviewboard/integrations/integration.py (Diff revision 9)
The issue has been resolved. Show all issues
```
No need for the None parameter. That's the default.
```

reviewboard/integrations/manager.py (Diff revision 9)

The issue has been resolved. Show all issues

I don't think we should have a load method, and we certainly shouldn't call it when initializing Review Board. It means an extra database query and setup work every time initialize is called, which happens often enough.

Instead, how about fetching integrations the first time they're needed for an Integration. That way, we don't have a database query unconditionally for all installs, but rather, only if there are integrations configured.

xuanyi April 5, 2015, 12:55 a.m.

I could make the integration to be fetch the first time they are needed for the initialization. However, currently the configured integration only initialize their services after it has been retrieve for the first time. What will then be a good way to ensure that all the configured integration's services are initialize when the server is starting up?

reviewboard/integrations/manager.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Combine these conditionals.
```

reviewboard/integrations/manager.py (Diff revision 9)

The issue has been resolved. Show all issues

This is pretty redundant. We should have one or the other.

reviewboard/integrations/models.py (Diff revision 9)
The issue has been resolved. Show all issues
```
"integration"
```

reviewboard/integrations/models.py (Diff revision 9)

The issue has been dropped. Show all issues

This can be one statement.

xuanyi April 5, 2015, 12:55 a.m.

Most of the reviewboard models which requires local_site used this instead of a single statement. Should I still keep this to be consistent with them?

reviewboard/integrations/tests.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Swap these (alphabetical order).
```
reviewboard/integrations/tests.py (Diff revision 9)
The issue has been resolved. Show all issues
```
""" on the next line.
```
reviewboard/integrations/tests.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Here too.
Same below.
```
reviewboard/testing/testcase.py (Diff revision 9)
The issue has been resolved. Show all issues
```
"Create"
```

Description:: This provides the integration base class, database model and manager for extensions to implement and register integrations.

~
Initial: February 8, 2015
~
Compared to the extension framework, these integration model will provide a way for users, such as local site administrator, to manage their own individiual integration. Third-party services will need to subclass the integration base class to provide the initialize and shutdown method for their services.

~
Added IntegrationHook in hooks.py
~
With the database model, multiple instances of the integration's configuration could be supported, and thus providing multiple configurations of the integration for a single web server.
-
Added integration class to be use as the base class for all integration
-
Added manager class to manage all integrations
-
Added ConfiguredIntegration model that stores the configuration of an integration
-
Registered integration in settings.py

~
Update: February 15, 2015
~
All these configured integration will then be managed through the integration manager.
-
-
Added unit tests for IntegrationManager
-
Added unit tests to extensions.test for IntegrationHook
-
Added fixtures
-
-
Update: March 29, 2015
-
-
Implemented common test method in testcase
-
Modified permission for models

Change Summary:

Fix review issues

Commit:

709a7e7548cddf0a611b3e01103c693fd118c992

90f5d8a4e3a602bfd4a77d1f4aa3304badadcb75

Diff:

Revision 10 (+562 -2)

Show changes

	reviewboard/__init__.py
	reviewboard/settings.py
	reviewboard/accounts/backends.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py
	reviewboard/integrations/__init__.py
	reviewboard/integrations/integration.py
	reviewboard/integrations/manager.py
	3 more

Tool: Pyflakes
Processed Files:
    reviewboard/testing/testcase.py
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/__init__.py
    reviewboard/accounts/backends.py
    reviewboard/integrations/models.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py

reviewboard/__init__.py (Diff revision 10)
The issue has been resolved. Show all issues
```
 'reviewboard' imported but unused
```
reviewboard/settings.py (Diff revision 10)
The issue has been resolved. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 10)
The issue has been resolved. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 10)
The issue has been resolved. Show all issues
```
 'PIPELINE_JS' imported but unused
```
reviewboard/settings.py (Diff revision 10)
The issue has been resolved. Show all issues
```
 'PIPELINE_CSS' imported but unused
```

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/testing/testcase.py
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/__init__.py
    reviewboard/accounts/backends.py
    reviewboard/integrations/models.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py

Testing Done:

~		Manual testing with a dummy integration.
	~	Tested with a dummy integration.

		Unit tests pass.

Commit:

90f5d8a4e3a602bfd4a77d1f4aa3304badadcb75

145dd6db6d769a0728b5b34f9d185b8e0467f013

Diff:

Revision 11 (+554 -2)

Show changes

	reviewboard/__init__.py
	reviewboard/settings.py
	reviewboard/accounts/backends.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py
	reviewboard/integrations/__init__.py
	reviewboard/integrations/integration.py
	reviewboard/integrations/manager.py
	3 more

Tool: Pyflakes
Processed Files:
    reviewboard/testing/testcase.py
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/__init__.py
    reviewboard/accounts/backends.py
    reviewboard/integrations/models.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/testing/testcase.py
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/__init__.py
    reviewboard/accounts/backends.py
    reviewboard/integrations/models.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py

reviewboard/__init__.py (Diff revision 11)
The issue has been resolved. Show all issues
```
 'get_integration_manager' imported but unused
```
reviewboard/__init__.py (Diff revision 11)
The issue has been dropped. Show all issues
```
 'reviewboard' imported but unused
```
reviewboard/settings.py (Diff revision 11)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 11)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 11)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 11)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```

Testing Done:

~		Tested with a dummy integration.
	~	Added unit test for the integrations.
	+
	+	Test manager on creating, deleting, toggling of configured integration.
	+	Test manager on handling of invalid configured integration.
	+
	+	Mannual testing with a dummy integration.

		Unit tests pass.

Commit:

145dd6db6d769a0728b5b34f9d185b8e0467f013

bdfd08f950d23f696b8776d8e033dc2b47fd965f

Diff:

Revision 12 (+553 -2)

Show changes

	reviewboard/settings.py
	reviewboard/accounts/backends.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py
	reviewboard/integrations/__init__.py
	reviewboard/integrations/integration.py
	reviewboard/integrations/manager.py
	reviewboard/integrations/models.py
	2 more

Tool: Pyflakes
Processed Files:
    reviewboard/testing/testcase.py
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/accounts/backends.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/testing/testcase.py
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/accounts/backends.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py

reviewboard/settings.py (Diff revision 12)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 12)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 12)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 12)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```

Commit:

bdfd08f950d23f696b8776d8e033dc2b47fd965f

d0bd6c60c3d6fe5226dfc01dd62c64c43fd3da05

Diff:

Revision 13 (+556 -2)

Show changes

	reviewboard/settings.py
	reviewboard/accounts/backends.py
	reviewboard/extensions/hooks.py
	reviewboard/extensions/tests.py
	reviewboard/integrations/__init__.py
	reviewboard/integrations/integration.py
	reviewboard/integrations/manager.py
	reviewboard/integrations/models.py
	2 more

Tool: Pyflakes
Processed Files:
    reviewboard/testing/testcase.py
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/accounts/backends.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/testing/testcase.py
    reviewboard/settings.py
    reviewboard/extensions/tests.py
    reviewboard/accounts/backends.py
    reviewboard/extensions/hooks.py
    reviewboard/integrations/models.py
    reviewboard/integrations/manager.py
    reviewboard/integrations/tests.py
    reviewboard/integrations/integration.py

Ignored Files:
    reviewboard/integrations/__init__.py

reviewboard/settings.py (Diff revision 13)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 13)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 13)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 13)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```

Fix it!

reviewboard/extensions/hooks.py (Diff revision 13)
An issue was opened. Show all issues
```
Imports should come in alphabetical order.
```
reviewboard/extensions/hooks.py (Diff revision 13)
An issue was opened. Show all issues
```
Should be in the imperative mood ("Wrap a callback ...")
```

reviewboard/extensions/hooks.py (Diff revision 13)

An issue was opened. Show all issues

exc_info should be True, rather than 1 (we have 1 a bunch of other places, but I'm slowly cleaning those up).

reviewboard/extensions/tests.py (Diff revision 13)

An issue was opened. Show all issues

This should use the same terminology as you changed above ("registration" rather than "initializing")

reviewboard/extensions/tests.py (Diff revision 13)

An issue was opened. Show all issues

This should use the same terminology as you changed above ("registration" rather than "initializing")

reviewboard/integrations/integration.py (Diff revision 13)
An issue was opened. Show all issues
```
Should be in the imperative mood ("Shut down...")
```
reviewboard/integrations/integration.py (Diff revision 13)
An issue was opened. Show all issues
```
Should be in the imperative mood ("Return the...")
```
reviewboard/integrations/integration.py (Diff revision 13)
An issue was opened. Show all issues
```
Should be in the imperative mood ("Return the...")
```
reviewboard/integrations/integration.py (Diff revision 13)
An issue was opened. Show all issues
```
Should be in the imperative mood ("Return the...")
```

reviewboard/integrations/manager.py (Diff revision 13)

An issue was opened. Show all issues

The logging methods will do a format operation internally, so you can pass in config_id as a parameter rather than formatting it yourself.

reviewboard/integrations/manager.py (Diff revision 13)
An issue was opened. Show all issues
```
Should be in the imperative mood ("Return all...")
```
reviewboard/integrations/manager.py (Diff revision 13)
An issue was opened. Show all issues
```
Should be in the imperative mood ("Return the...")
```
reviewboard/integrations/manager.py (Diff revision 13)
An issue was opened. Show all issues
```
Should be in the imperative mood ("Return the...")
```
reviewboard/integrations/models.py (Diff revision 13)
An issue was opened. Show all issues
```
Should be in the imperative mood ("Return...")
```
reviewboard/integrations/models.py (Diff revision 13)
An issue was opened. Show all issues
```
Should be in the imperative mood ("Return...")
```

Status:: Discarded
Change Summary:: Closed in favor of a revised version of Integrations that landed for 3.0.

		This provides the integration base class, database model and manager for extensions to implement and register integrations.

~		Initial: February 8, 2015
	~	Compared to the extension framework, these integration model will provide a way for users, such as local site administrator, to manage their own individiual integration. Third-party services will need to subclass the integration base class to provide the initialize and shutdown method for their services.

~		Added IntegrationHook in hooks.py
	~	With the database model, multiple instances of the integration's configuration could be supported, and thus providing multiple configurations of the integration for a single web server.
-		Added integration class to be use as the base class for all integration
-		Added manager class to manage all integrations
-		Added ConfiguredIntegration model that stores the configuration of an integration
-		Registered integration in settings.py

~		Update: February 15, 2015
	~	All these configured integration will then be managed through the integration manager.
-
-		Added unit tests for IntegrationManager
-		Added unit tests to extensions.test for IntegrationHook
-		Added fixtures
-
-		Update: March 29, 2015
-
-		Implemented common test method in testcase
-		Modified permission for models