> On March 11, 2016, 1:30 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleStatus.java, > > lines 93-94 > > <https://reviews.apache.org/r/44492/diff/4/?file=1294961#file1294961line93> > > > > This changes existing behavior; might not be what we want. The > > HOLDING_FAILED status is not technically a failure. > > > > Upgrades and other areas may use this enum. If they do, we should not > > change it. I'd just create a new one to old what you need. > > > > Imagine upgrades starting to throw failures and aborting every time it > > hit a HOLDING_FAILED state.
I'll add my own enumset. > On March 11, 2016, 1:30 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummary.java, > > line 25 > > <https://reviews.apache.org/r/44492/diff/4/?file=1294967#file1294967line25> > > > > This class is still basically assuming a failure. The upgrade summary > > could be a success, no? I'm keeping the name generic for now. The first use case is to report an error. If that changes in the future, we can always add more properties. > On March 11, 2016, 1:30 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummaryResourceProvider.java, > > lines 157-178 > > <https://reviews.apache.org/r/44492/diff/4/?file=1294968#file1294968line157> > > > > This class is still basically assuming a failure. The upgrade summary > > could be a success, no? > > > > If we're saying that the upgrade summary is only going to be for a > > failure, then we should rename it to reflect that. > > > > Personally, I'm fine leaving it and populating the fields with success > > information if it finished. If there's no failed_reason they can assume success. I only want to add the minimal set of properties I know are needed right now. - Alejandro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44492/#review123134 ----------------------------------------------------------- On March 10, 2016, 11:12 p.m., Alejandro Fernandez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44492/ > ----------------------------------------------------------- > > (Updated March 10, 2016, 11:12 p.m.) > > > Review request for Ambari, Dmytro Grinenko, Dmitro Lisnichenko, Jonathan > Hurley, Nate Cole, and Sid Wagle. > > > Bugs: AMBARI-15330 > https://issues.apache.org/jira/browse/AMBARI-15330 > > > Repository: ambari > > > Description > ------- > > During RU/EU, need a way to bubble up an error of the current item that > failed. This is useful to quickly get a human-readable error that others UIs > can quickly retrieve. > It can print a human-readable error, plus stdout and stderr. > This would become part of the upgrade endpoint. e.g, > api/v1/clusters/$name/upgrade_summary/$request_id > > ``` > { > cluster_name: "c1", > request_id: 1, > fail_reason: "Failed calling RESTART ZOOKEEPER/ZOOKEEPER_SERVER on host > c6401.ambari.apache.org", > > // Notice that the rest are inherited from the failed task if it exists. > failed_task: { > attempt_cnt: 1, > command: "CUSTOM_COMMAND", > command_detail: "RESTART ZOOKEEPER/ZOOKEEPER_SERVER", > custom_command_name: "RESTART", > end_time: -1, > error_log: "/var/lib/ambari-agent/data/errors-1234.txt", > exit_code: 1, > host_name: "c6401.ambari.apache.org", > id: 1234, > output_log: "/var/lib/ambari-agent/data/output-1234.txt", > role: "ZOOKEEPER_SERVER", > stage_id: 1, > start_time: 123456789, > status: "HOLDING_FAILED", > stdout: "", > stderr: "" > } > } > ``` > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleStatus.java > 52523c7 > > ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java > 3526e23 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java > 7200b83 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeSummaryService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java > d1d3fe6 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/TaskResourceProvider.java > 510d6fb > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummary.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeSummaryResourceProvider.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java > e79f300 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java > b48ffa8 > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostRoleCommandEntity.java > 1674175 > > ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java > 2ac4d25 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeSummaryResourceProviderTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/44492/diff/ > > > Testing > ------- > > Verified on RU and EU by introducing a failure. > Waiting for unit test results. > > > Thanks, > > Alejandro Fernandez > >
