Skip to content
This repository has been archived by the owner on Aug 29, 2018. It is now read-only.

Remove double warning msg when adding cartridges #6369

Merged
merged 1 commit into from Mar 1, 2016

Conversation

sallyom
Copy link
Contributor

@sallyom sallyom commented Feb 27, 2016

bz1169690
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1169690

When adding cartridges to an app that is approaching quota limits,
the "Warning: Gear is using of disk quota" is printed twice.
This PR removes the report_quota when installing a cartridge, so the
quota is only reported once after the cartridge is installed.

@sallyom
Copy link
Contributor Author

sallyom commented Feb 27, 2016

[test]

@sallyom sallyom force-pushed the bz1531985890ae5fa7 branch 2 times, most recently from 3b977d0 to 9dc1288 Compare February 29, 2016 16:07
report_quota(output, args['--with-container-uuid'])
else
# do not print quota warning (output) during action "configure", to avoid double warning msg
report_quota('', args['--with-container-uuid'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we aren't going to do anything with the output, why run report_quota at all? It doesn't appear to do anything other than generate output, so it seems needless to run it in this case.

Also, to ensure that quota is reported with everything but the 'configure' step, why not use if action.to_s != "configure" ?

@sallyom
Copy link
Contributor Author

sallyom commented Feb 29, 2016

@tiwillia all set? thanks

@@ -159,7 +159,6 @@ def test_cartridge_do_action_quota
@agent.expects(:request).returns(request).times(3)

@agent.expects(:execute_action).with('test_agent', any_parameters).returns [0, 'test output', nil]
@agent.expects(:report_quota).with(instance_of(String), 'uuid')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why this was removed? It looks like the test uses the action 'test_agent', which should match your conditional and report quota.

bz1169690
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1169690

When adding cartridges to an app that is approaching quota limits,
the "Warning: Gear <uuid> is using <percent> of disk quota" is printed twice.
This PR removes the report_quota when installing a cartridge, so the
quota is only reported once after the cartridge is installed.
@openshift-bot
Copy link

Evaluated for online test up to dc0f7c9

@openshift-bot
Copy link

@sallyom
Copy link
Contributor Author

sallyom commented Mar 1, 2016

@tiwillia fixed up, thank you

@tiwillia
Copy link
Member

tiwillia commented Mar 1, 2016

LGTM. In testing, I can see that only one warning message is seen when adding a cartridge.

[merge]

@openshift-bot
Copy link

Online Merge Results: SUCCESS (https://ci.dev.openshift.redhat.com/jenkins/job/merge_pull_requests/6716/) (Image: devenv_5777)

@openshift-bot
Copy link

Evaluated for online merge up to dc0f7c9

openshift-bot pushed a commit that referenced this pull request Mar 1, 2016
@openshift-bot openshift-bot merged commit 6398370 into openshift:master Mar 1, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants