Bug 536186 - (RHQ-563) DeployPackagesResponse should let me change results
DeployPackagesResponse should let me change results
Product: RHQ Project
Classification: Other
Component: Content (Show other bugs)
All All
medium Severity medium (vote)
: ---
: ---
Assigned To: Jason Dobies
: Improvement
Depends On:
  Show dependency treegraph
Reported: 2008-06-11 00:40 EDT by John Mazzitelli
Modified: 2008-06-11 09:45 EDT (History)
0 users

See Also:
Fixed In Version: 1.0.1
Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of:
Last Closed:
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description John Mazzitelli 2008-06-11 00:40:00 EDT
DeployPackagesResponse takes a result in the constructor. But there is no setter.  So I can't create one initally, set the result to SUCCESS and later if I determine an error occurred, I can't flip it to FAILURE.  There *are* setters to set the error message - so it would make sense to let me change the results. It would make using the response object easier. Here's how I think you shoudl be able to use this object - notice I build it up as I go along iteratng thru the packages to be deployed:

DeployPackagesResponse r = new DeployPackagesResponse(key, SUCCESS);
for each package to deploy()
    try {
            ... deploy package....
    catch (Exception e) {
        r.setResult(FAILURE); // see below- shoudln't really need to do this anyway)

another alternative is to have the setErrorMessage methods implicitly change the result to FAILURE. In fact, it should do this anyway.  If I create a response with SUCCESS but later set an error message, it doesn't make sense that the result remain SUCCESS.  Setting an error message should implicitly assume the result was a FAILURE. If it works like this, I don't see a setter to set the results (though it might still make the api nicer).

I actually don't see how you easily work with this response object as it is now.  You can't set a bulk collection of individual package responses (i.e. this is missing an API like "addAllPackageResponses()", so how do you easily build up a DeployPackagesResponse without forcing to know what the end result is before hand (which you need because you can only set the result via the constructor).
Comment 1 John Mazzitelli 2008-06-11 00:48:18 EDT
ugh - same problem with DeployIndividualPackageResponse.

This API is much harder to work with than it should be.

Need to do one or both of:

1) provide a setContentResponseResult() method
2) have setErrorMessage methods implicitly set the result to FAILURE
Comment 2 John Mazzitelli 2008-06-11 00:58:13 EDT
same problems with RemovePackageResponse/RemoveIndividualPackageResponse
Comment 3 Jason Dobies 2008-06-11 09:45:11 EDT
r960 - DeployPackagesResponse.java, DeployIndividualPackageResponse.java, RemoveIndividualPackageResponse.java, RemovePackagesResponse.java
- Loosened up result code setting restrictions by adding setter and constructor that doesn't require it.
Comment 4 Red Hat Bugzilla 2009-11-10 16:11:59 EST
This bug was previously known as http://jira.rhq-project.org/browse/RHQ-563

Note You need to log in before you can comment on or make changes to this bug.