Install an extension on-the-fly from a remote source.

Review Request #3973 — Created March 18, 2013 and submitted

Information

Djblets
master

Reviewers

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 """

chipx86chipx86

"containing"

chipx86chipx86

We should ideally wrap the errors in some sane type they can catch, or otherwise, list every error they need …

chipx86chipx86

This looks like it can fit on one line.

chipx86chipx86

Col: 49 E127 continuation line over-indented for visual indent

reviewbotreviewbot

Col: 41 E127 continuation line over-indented for visual indent

reviewbotreviewbot

Col: 80 E501 line too long (80 > 79 characters)

reviewbotreviewbot

Col: 41 E127 continuation line over-indented for visual indent

reviewbotreviewbot

"pkg_resources" Can you make these bullet point lists?

chipx86chipx86

What does this mean? We should never be at a point where there's an uncaught exception causing an HTTP 500.

chipx86chipx86

Can you make this align with the others? (Ignore pep8 here.)

chipx86chipx86

Col: 49 E127 continuation line over-indented for visual indent

reviewbotreviewbot

Alignment issue.

chipx86chipx86

Col: 41 E127 continuation line over-indented for visual indent

reviewbotreviewbot

Col: 41 E127 continuation line over-indented for visual indent

reviewbotreviewbot

Second sentence is a little awkward. How about: "Installation may fail if a malformed install_url or package_name is passed, which …

mike_conleymike_conley

Col: 22 E222 multiple spaces after operator

reviewbotreviewbot

For consistency's sake, this = should be aligned with all of the others.

mike_conleymike_conley

Col: 27 E222 multiple spaces after operator

reviewbotreviewbot

Col: 20 E221 multiple spaces before operator

reviewbotreviewbot

Col: 25 E221 multiple spaces before operator

reviewbotreviewbot
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        djblets/extensions/base.py
      Ignored Files:
    
    
  2. 
      
chipx86
  1. Any luck figuring out the .pth file override issue?
  2. djblets/extensions/base.py (Diff revision 1)
     
     
    Show all issues
    No space after """
  3. djblets/extensions/base.py (Diff revision 1)
     
     
    Show all issues
    "containing"
  4. djblets/extensions/base.py (Diff revision 1)
     
     
    Show all issues
    We should ideally wrap the errors in some sane type they can catch, or otherwise, list every error they need to worry about. Right now, it requires digging into the source (and I actually have no idea what those errors are).
  5. djblets/extensions/base.py (Diff revision 1)
     
     
     
    Show all issues
    This looks like it can fit on one line.
  6. 
      
AY
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        djblets/extensions/base.py
        djblets/webapi/errors.py
      Ignored Files:
    
    
  2. djblets/webapi/errors.py (Diff revision 2)
     
     
    Show all issues
    Col: 49
     E127 continuation line over-indented for visual indent
    
  3. djblets/webapi/errors.py (Diff revision 2)
     
     
    Show all issues
    Col: 41
     E127 continuation line over-indented for visual indent
    
  4. djblets/webapi/errors.py (Diff revision 2)
     
     
    Show all issues
    Col: 80
     E501 line too long (80 > 79 characters)
    
  5. djblets/webapi/errors.py (Diff revision 2)
     
     
    Show all issues
    Col: 41
     E127 continuation line over-indented for visual indent
    
  6. 
      
AY
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        djblets/extensions/base.py
        djblets/webapi/errors.py
      Ignored Files:
    
    
  2. djblets/webapi/errors.py (Diff revision 3)
     
     
    Show all issues
    Col: 49
     E127 continuation line over-indented for visual indent
    
  3. djblets/webapi/errors.py (Diff revision 3)
     
     
    Show all issues
    Col: 41
     E127 continuation line over-indented for visual indent
    
  4. djblets/webapi/errors.py (Diff revision 3)
     
     
    Show all issues
    Col: 41
     E127 continuation line over-indented for visual indent
    
  5. 
      
chipx86
  1. 
      
  2. djblets/extensions/base.py (Diff revision 3)
     
     
    Show all issues
    "pkg_resources"
    
    Can you make these bullet point lists?
  3. djblets/extensions/base.py (Diff revision 3)
     
     
    Show all issues
    What does this mean? We should never be at a point where there's an uncaught exception causing an HTTP 500.
  4. djblets/webapi/errors.py (Diff revision 3)
     
     
    Show all issues
    Can you make this align with the others? (Ignore pep8 here.)
  5. djblets/webapi/errors.py (Diff revision 3)
     
     
    Show all issues
    Alignment issue.
  6. 
      
AY
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        djblets/extensions/base.py
        djblets/webapi/errors.py
      Ignored Files:
    
    
  2. djblets/webapi/errors.py (Diff revision 4)
     
     
    Show all issues
    Col: 22
     E222 multiple spaces after operator
    
  3. djblets/webapi/errors.py (Diff revision 4)
     
     
    Show all issues
    Col: 27
     E222 multiple spaces after operator
    
  4. 
      
mike_conley
  1. 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
  2. djblets/extensions/base.py (Diff revision 4)
     
     
     
     
    Show all issues
    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."
  3. djblets/webapi/errors.py (Diff revision 4)
     
     
    Show all issues
    For consistency's sake, this = should be aligned with all of the others.
  4. 
      
AY
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        djblets/extensions/base.py
        djblets/webapi/errors.py
      Ignored Files:
    
    
  2. djblets/webapi/errors.py (Diff revision 5)
     
     
    Show all issues
    Col: 20
     E221 multiple spaces before operator
    
  3. djblets/webapi/errors.py (Diff revision 5)
     
     
    Show all issues
    Col: 25
     E221 multiple spaces before operator
    
  4. 
      
mike_conley
  1. Looks good to me! Ship it!
  2. 
      
chipx86
  1. Ship It!
  2. 
      
AY
Review request changed
Status:
Completed
Change Summary:
Pushed to master (fc5f39f)