Skip to content

Conversation

@elek
Copy link
Member

@elek elek commented Dec 13, 2019

  • DatanodeAdminMonitorInterface has some settesr and getters but they are not part of the contract they are used by the implementation. An other implementation of the interface may require different setters. Therefore the interface should contain only the required fields.
  • DatanodeAdminMonitorInterface can be renamed to DatanodeAdminMonitor to follow the naming convention in the code (*Impl)
  • PipelineManager is unused field can be removed from DatanodeAdminMonitorImpl

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-2728

How was this patch tested?

With the existing Unit tests.

@elek elek requested a review from sodonnel December 13, 2019 12:27
@elek
Copy link
Member Author

elek commented Dec 13, 2019

@sodonnel please review.

@sodonnel
Copy link
Contributor

This change LGTM and is what we discussed previously. I assume no logic has changed in DatanodeAdminMonitorImpl aside from the constructor, dropped the getters and removing the unused field? As the file was renamed the diff just shows it as all new code.

Copy link
Contributor

@sodonnel sodonnel left a comment

Choose a reason for hiding this comment

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

This change LGTM and is what we discussed previously.

@sodonnel sodonnel merged commit f66c40c into apache:HDDS-1880-Decom Dec 13, 2019
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.

2 participants