Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(4428)

Issue 15400052: Fix export and import bugs

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 6 months ago by gary.poster
Modified:
10 years, 6 months ago
Reviewers:
mp+192286, jeff.pihach
Visibility:
Public.

Description

Fix export and import bugs Our support for exporting and importing config values and expose flags was buggy. This fixes four discrete but related bugs. - Exporting config values would exclude those that had false-y default values. - We did not export the expose flag. - Importing config values did not work. - We called the expose flag "exposed", which is not what the actual delpoyer expects. To qa, open up the sandbox, deploy apache with "enable_modules" set to something or other, and expose it. Then export the environment. When you look at the service in the file, you should see "expose: true" and "enable_modules: mod_proxy" or whatever value you set. Then reload the GUI and drag the file into the sandbox. When the service has loaded, you should see that it is exposed, and when you look at the config in the inspector, you should see your value in the "enable_modules" field. https://code.launchpad.net/~gary/juju-gui/exportBugs/+merge/192286 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2

Patch Set 2 : Fix export and import bugs #

Unified diffs Side-by-side diffs Delta from patch set Stats (+76 lines, -12 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M app/models/models.js View 2 chunks +7 lines, -3 lines 0 comments Download
M app/store/env/fakebackend.js View 3 chunks +7 lines, -3 lines 0 comments Download
M test/data/wp-deployer.yaml View 2 chunks +2 lines, -5 lines 0 comments Download
M test/test_fakebackend.js View 2 chunks +12 lines, -1 line 0 comments Download
M test/test_model.js View 1 chunk +46 lines, -0 lines 0 comments Download

Messages

Total messages: 3
gary.poster
Please take a look.
10 years, 6 months ago (2013-10-23 05:35:07 UTC) #1
jeff.pihach
Thanks for this fix! LGTM QA OK! https://codereview.appspot.com/15400052/diff/1/app/models/models.js File app/models/models.js (right): https://codereview.appspot.com/15400052/diff/1/app/models/models.js#newcode1193 app/models/models.js:1193: if (service.get('exposed')) ...
10 years, 6 months ago (2013-10-23 16:21:14 UTC) #2
gary.poster
10 years, 6 months ago (2013-10-23 16:33:06 UTC) #3
*** Submitted:

Fix export and import bugs

Our support for exporting and importing config values and expose flags was
buggy.  This fixes four discrete but related bugs.

 - Exporting config values would exclude those that had false-y default values.
 - We did not export the expose flag.
 - Importing config values did not work.
 - We called the expose flag "exposed", which is not what the actual delpoyer
expects.

To qa, open up the sandbox, deploy apache with "enable_modules" set to something
or other, and expose it.  Then export the environment.  When you look at the
service in the file, you should see "expose: true" and "enable_modules:
mod_proxy" or whatever value you set.  Then reload the GUI and drag the file
into the sandbox.  When the service has loaded, you should see that it is
exposed, and when you look at the config in the inspector, you should see your
value in the "enable_modules" field.

R=jeff.pihach
CC=
https://codereview.appspot.com/15400052
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b