Install an extension on-the-fly from a remote source.
Review Request #3973 — Created March 18, 2013 and submitted
Added a method to djblet's extension manager that installs an extension, given a remote URL with the extension egg and package name. This modification is required as a pre-requisite, for the upcoming extension browser module to ReviewBoard. There could be possible errors such as (but not limited to): The URL being a 404, incorrect package_name, incompatible egg or the package being already present. These would be thrown as exceptions and have to be handled by the caller appropriately.
The changes have been tested with the different variations of extension browser module so far. Presently, with the module being absent it can tested manually by invoking the install_extension() method on the extension manager with the required parameters. If the extension isn't already installed, the installation would go through.
Description | From | Last Updated |
---|---|---|
No space after """ |
chipx86 | |
"containing" |
chipx86 | |
We should ideally wrap the errors in some sane type they can catch, or otherwise, list every error they need … |
chipx86 | |
This looks like it can fit on one line. |
chipx86 | |
Col: 49 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 41 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 80 E501 line too long (80 > 79 characters) |
reviewbot | |
Col: 41 E127 continuation line over-indented for visual indent |
reviewbot | |
"pkg_resources" Can you make these bullet point lists? |
chipx86 | |
What does this mean? We should never be at a point where there's an uncaught exception causing an HTTP 500. |
chipx86 | |
Can you make this align with the others? (Ignore pep8 here.) |
chipx86 | |
Col: 49 E127 continuation line over-indented for visual indent |
reviewbot | |
Alignment issue. |
chipx86 | |
Col: 41 E127 continuation line over-indented for visual indent |
reviewbot | |
Col: 41 E127 continuation line over-indented for visual indent |
reviewbot | |
Second sentence is a little awkward. How about: "Installation may fail if a malformed install_url or package_name is passed, which … |
mike_conley | |
Col: 22 E222 multiple spaces after operator |
reviewbot | |
For consistency's sake, this = should be aligned with all of the others. |
mike_conley | |
Col: 27 E222 multiple spaces after operator |
reviewbot | |
Col: 20 E221 multiple spaces before operator |
reviewbot | |
Col: 25 E221 multiple spaces before operator |
reviewbot |
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: djblets/extensions/base.py djblets/webapi/errors.py Ignored Files:
-
-
-
-
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: djblets/extensions/base.py djblets/webapi/errors.py Ignored Files:
-
-
-
-
Hey Surya, I'm pretty happy with this (although I haven't been down in the guts of setuptools like you and ChipX86 in a while, so take my opinion with a grain of salt). Just two little style / doc nits, but otherwise, I think this looks really reasonable. -Mike
-
Second sentence is a little awkward. How about: "Installation may fail if a malformed install_url or package_name is passed, which will cause an InstallExtensionError exception to be raised."
-