Skip to content

Conversation

@benyoka
Copy link
Contributor

@benyoka benyoka commented May 24, 2018

What changes were proposed in this pull request?

Fixed NPE in exporting caused by recent changes on the branch.

How was this patch tested?

  1. Tested blueprint export manually
  2. Fixed broken unit tests in ClusterBlueprintRendererTest
  3. Unit test results for ambari-server: Tests run: 4793, Failures: 35, Errors: 219, Skipped: 91

Balazs Bence Sari added 21 commits February 21, 2018 17:01
…nto AMBARI-23032-branch-feature-AMBARI-14714
…nto AMBARI-23032-branch-feature-AMBARI-14714
…nto AMBARI-23032-branch-feature-AMBARI-14714
…nto AMBARI-23032-branch-feature-AMBARI-14714
…nto AMBARI-23032-branch-feature-AMBARI-14714
…nto AMBARI-23032-branch-feature-AMBARI-14714
@benyoka benyoka requested review from adoroszlai and rnettleton May 24, 2018 11:04
@asfgit
Copy link

asfgit commented May 24, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/2439/
Test FAILed.
Test FAILured.

serviceGroupToMpack = clusterNode.getChild("servicegroups").getChildren().stream().
map(tn -> tn.getObject().getPropertiesMap().get(ServiceGroupResourceProvider.RESPONSE_KEY)).
collect(toMap(m -> m.get("service_group_name").toString(), m -> new StackId(m.get("version").toString())));
collect(toMap(m -> m.get("service_group_name").toString(), m -> new StackId(m.get("stack").toString())));
Copy link
Contributor

Choose a reason for hiding this comment

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

#1363 is changing service group response in the opposite way (return "version" instead of "stack"). I think these strings should be defined in a constant in ServiceGroupResourceProvider (also "mpack_name" etc.).

Copy link

@rnettleton rnettleton left a comment

Choose a reason for hiding this comment

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

The patch looks fine to me.

Just a single comment that needs to be addressed, since it looks like there might be a error in the Component.equals() implementation.

return Objects.equals(name, component.name) &&
Objects.equals(stackId, component.stackId) &&
Objects.equals(mpackInstance, component.mpackInstance) &&
Objects.equals(stackId, component.stackId) &&

Choose a reason for hiding this comment

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

Why is the same "Objects.equals(stackId, component.stackId)" line added in two places in this equals() implementation? Is this a typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like a merge issue. Thanks for spotting it.

@asfgit
Copy link

asfgit commented May 24, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/2440/
Test FAILed.
Test FAILured.

@benyoka benyoka merged commit 7bbf158 into apache:branch-feature-AMBARI-14714 May 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants