Skip to content

RATIS-1547. Make GrpcClientProtocolService and log to public#617

Closed
captainzmc wants to merge 1 commit intoapache:masterfrom
captainzmc:1547
Closed

RATIS-1547. Make GrpcClientProtocolService and log to public#617
captainzmc wants to merge 1 commit intoapache:masterfrom
captainzmc:1547

Conversation

@captainzmc
Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

see: https://issues.apache.org/jira/browse/RATIS-1547

Comment on lines +51 to +52
public class GrpcClientProtocolService extends RaftClientProtocolServiceImplBase {
public static final Logger LOG = LoggerFactory.getLogger(GrpcClientProtocolService.class);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@captainzmc , sorry that GrpcClientProtocolService is not a public API. We should not change it for an Ozone unit test. We should fix the test instead.

class GrpcClientProtocolService extends RaftClientProtocolServiceImplBase {
private static final Logger LOG = LoggerFactory.getLogger(GrpcClientProtocolService.class);
public class GrpcClientProtocolService extends RaftClientProtocolServiceImplBase {
public static final Logger LOG = LoggerFactory.getLogger(GrpcClientProtocolService.class);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I also encountered compile error while trying recent Ratis changes (RATIS-1515, etc.) with Ozone. But I think exposing LOG as public only for Ozone test is bad practice. We may be able to improve TestRatisPipelineLeader by using custom retry policy.

@captainzmc
Copy link
Copy Markdown
Member Author

Thanks @szetszwo and @adoroszlai for the suggestion. Agree with you, let’s improve TestRatisPipelineLeader to avoid this error.

@captainzmc captainzmc closed this Mar 3, 2022
@szetszwo
Copy link
Copy Markdown
Contributor

@captainzmc , filed https://issues.apache.org/jira/browse/HDDS-6461 for updating Ratis snapshot version including fixing TestRatisPipelineLeader; see apache/ozone#3205

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.

3 participants