Merge lp://staging/~jkakar/landscape-client/include-eucalyptus-by-default into lp://staging/~landscape/landscape-client/trunk

Proposed by Jamu Kakar
Status: Merged
Approved by: Kevin McDermott
Approved revision: 278
Merged at revision: 287
Proposed branch: lp://staging/~jkakar/landscape-client/include-eucalyptus-by-default
Merge into: lp://staging/~landscape/landscape-client/trunk
Diff against target: 45 lines (+7/-6)
3 files modified
landscape/manager/config.py (+1/-1)
landscape/manager/eucalyptus.py (+1/-1)
landscape/manager/tests/test_config.py (+5/-4)
To merge this branch: bzr merge lp://staging/~jkakar/landscape-client/include-eucalyptus-by-default
Reviewer Review Type Date Requested Status
Kevin McDermott (community) Approve
Thomas Herve (community) Approve
Free Ekanayaka (community) Approve
Review via email: mp+31977@code.staging.launchpad.net

Description of the change

This branch introduces the following changes:

- The Eucalyptus plugin is loaded by default.

To post a comment you must log in.
Revision history for this message
Free Ekanayaka (free.ekanayaka) wrote :

Looks good. +1!

review: Approve
Revision history for this message
Kevin McDermott (bigkevmcd) wrote :

2010-08-13 11:38:20,718 ERROR [MainThread] No module named imagestore.lib.service
Traceback (most recent call last):
  File "./landscape/manager/eucalyptus.py", line 141, in send_message
    service_hub = self._service_hub_factory(data_path)
  File "./landscape/manager/eucalyptus.py", line 219, in start_service_hub
    from imagestore.lib.service import ServiceHub
ImportError: No module named imagestore.lib.service
2010-08-13 11:38:20,719 INFO [MainThread] Couldn't start service hub. 'eucalyptus-info' plugin has been disabled.

I don't think we should allow this traceback to appear in the logs, and it's unlikely that we want imagestore to become a dependency of landscape-client ?

review: Needs Fixing
Revision history for this message
Jamu Kakar (jkakar) wrote :

Kevin:

imagestore is not a hard dependency of landscape-client. It is only
needed when the Eucalyptus plugin is run. The traceback is slightly
unfortunate, but the behaviour is correct: the plugin is disabling
itself because the imagestore package isn't available.

Revision history for this message
Thomas Herve (therve) wrote :

+1!

review: Approve
Revision history for this message
Jamu Kakar (jkakar) wrote :

Kevin:

Is your comment a blocker still? ie: Have I missed an important
detail or is the branch fine to land now?

Revision history for this message
Thomas Herve (therve) wrote :

Jamu: yes, please turn that traceback into a debug output.

Revision history for this message
Jamu Kakar (jkakar) wrote :

Okay, fixed. The exception is now written to the log with
logging.debug.

277. By Jamu Kakar

- Merged trunk.

278. By Jamu Kakar

- When an exception causes the eucalyptus-info plugin to be disabled
  information about it will be written to the log using
  logging.debug instead of logging.exception. This prevents a
  traceback from appearing in the log during normal client operation.

Revision history for this message
Kevin McDermott (bigkevmcd) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: