Merge lp://qastaging/~psivaa/charms/precise/ubuntu-ci-services-itself/upstart-with-proxy-2 into lp://qastaging/~canonical-ci-engineering/charms/precise/ubuntu-ci-services-itself/upstart

Proposed by Para Siva
Status: Needs review
Proposed branch: lp://qastaging/~psivaa/charms/precise/ubuntu-ci-services-itself/upstart-with-proxy-2
Merge into: lp://qastaging/~canonical-ci-engineering/charms/precise/ubuntu-ci-services-itself/upstart
Diff against target: 47 lines (+15/-1)
2 files modified
config.yaml (+8/-0)
hooks/hooks.py (+7/-1)
To merge this branch: bzr merge lp://qastaging/~psivaa/charms/precise/ubuntu-ci-services-itself/upstart-with-proxy-2
Reviewer Review Type Date Requested Status
Andy Doan (community) Needs Fixing
Review via email: mp+220333@code.qastaging.launchpad.net

Commit message

This is for ppa-cleaner upstart job to take in http proxies.

Description of the change

This is for ppa-cleaner upstart job to take in http proxies.

To post a comment you must log in.
Revision history for this message
Para Siva (psivaa) wrote :

Just realised the need for this to work with uci-engine trunk. Still WIP

Revision history for this message
Para Siva (psivaa) wrote :

This should work for both private clouds and otherwise. The change look cumbersome. Please feel free to correct.

Revision history for this message
Andy Doan (doanac) wrote :

You should be able to keep this diff smaller with jinja2 conditional logic like:

{% if proxy_url %}
env http_proxy={proxy_url}
env https_proxy={proxy_url}
{% endif %}

Then you won't need those two big blocks that are essentially the same logic

review: Needs Fixing
11. By Para Siva

Better conditional format

12. By Para Siva

Upstart command to work with both type of clouds-fixed

13. By Para Siva

Typo fix

Revision history for this message
Para Siva (psivaa) wrote :

Thanks for that doanac. Direct copying of that snippet is throwing KeyError during the charm installation and therefore I've used the fix in r13. This atleast does not throw errors during deployments. I've tried to verify if that'd give me the needed upstart job but it's taking too long to get one deployment going. Will update once i've been able to deploy.

Revision history for this message
Andy Doan (doanac) wrote :

small comment to make it better

14. By Para Siva

Using doanac's hack for conditional proxy

15. By Para Siva

Adding no-proxy for the needed urls

16. By Para Siva

Fix the syntax in no proxy inclusion

Unmerged revisions

16. By Para Siva

Fix the syntax in no proxy inclusion

15. By Para Siva

Adding no-proxy for the needed urls

14. By Para Siva

Using doanac's hack for conditional proxy

13. By Para Siva

Typo fix

12. By Para Siva

Upstart command to work with both type of clouds-fixed

11. By Para Siva

Better conditional format

10. By Para Siva

Making the hooks conditional if the proxy is defined

9. By Para Siva

Pep8 fix

8. By Para Siva

Add http proxy env variable for ppa-cleaner upstart job

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