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

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

ayrus
Djblets
master
djblets
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)
     
     
    No space after """
  3. djblets/extensions/base.py (Diff revision 1)
     
     
    "containing"
  4. djblets/extensions/base.py (Diff revision 1)
     
     
    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)
     
     
     
    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)
     
     
    Col: 49
     E127 continuation line over-indented for visual indent
    
  3. djblets/webapi/errors.py (Diff revision 2)
     
     
    Col: 41
     E127 continuation line over-indented for visual indent
    
  4. djblets/webapi/errors.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  5. djblets/webapi/errors.py (Diff revision 2)
     
     
    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)
     
     
    Col: 49
     E127 continuation line over-indented for visual indent
    
  3. djblets/webapi/errors.py (Diff revision 3)
     
     
    Col: 41
     E127 continuation line over-indented for visual indent
    
  4. djblets/webapi/errors.py (Diff revision 3)
     
     
    Col: 41
     E127 continuation line over-indented for visual indent
    
  5. 
      
chipx86
  1. 
      
  2. djblets/extensions/base.py (Diff revision 3)
     
     
    "pkg_resources"
    
    Can you make these bullet point lists?
  3. djblets/extensions/base.py (Diff revision 3)
     
     
    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)
     
     
    Can you make this align with the others? (Ignore pep8 here.)
  5. djblets/webapi/errors.py (Diff revision 3)
     
     
    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)
     
     
    Col: 22
     E222 multiple spaces after operator
    
  3. djblets/webapi/errors.py (Diff revision 4)
     
     
    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)
     
     
     
     
    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)
     
     
    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)
     
     
    Col: 20
     E221 multiple spaces before operator
    
  3. djblets/webapi/errors.py (Diff revision 5)
     
     
    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: Closed (submitted)

Change Summary:

Pushed to master (fc5f39f)
Loading...