-
Notifications
You must be signed in to change notification settings - Fork 3.9k
api: Javadoc changes for io.grpc.LoadBalancer method signatures #11623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 29 commits
ab97045
fef4c92
778cfb4
328bcbf
6a66054
4113845
09c3509
62a88ec
f658685
c51a5e4
02e92c5
4c31880
9ddcb09
596d11d
d9bf85e
401a7d2
f388ac2
6e4efc7
cb315f0
18b74c0
380345c
846eb00
ee36c4e
b1f857f
d236b88
891b547
6d3abf0
ca3e613
f9ffbc6
47002fe
cb85563
b44e402
8bfedb0
b9cced0
66a3495
45d3c75
b67b7ab
1b0ddaa
cfaab92
a0e5bf2
35f0769
bddc31c
b9a49a0
1e6fef9
1674b6d
60b4a39
8b02cd4
5ece923
d746ef8
516e75b
36ecadb
aa43806
d756690
d2e6f04
2ab1ae2
4a690a2
f66c084
8c75ed0
60932d8
4b542f7
9fc5391
af00e50
ea42960
226301c
e6bea7f
fd5263f
67c504f
29fd776
4300013
536311d
1ada412
9a63a50
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -156,15 +156,16 @@ public String toString() { | |
private int recursionCount; | ||
|
||
/** | ||
* Handles newly resolved server groups and metadata attributes from name resolution system. | ||
* {@code servers} contained in {@link EquivalentAddressGroup} should be considered equivalent | ||
* but may be flattened into a single list if needed. | ||
* Handles newly resolved addresses and metadata attributes from name resolution system. | ||
* {@link EquivalentAddressGroup} addresses should be considered equivalent but may be flattened | ||
* into a single list if needed. | ||
* | ||
* <p>Implementations should not modify the given {@code servers}. | ||
* <p>Implementations should not modify the given {@code resolvedAddresses}. | ||
ejona86 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* | ||
* @param resolvedAddresses the resolved server addresses, attributes, and config. | ||
* @since 1.21.0 | ||
*/ | ||
@Deprecated | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a javadoc on Deprecated and what to use instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Included in Javadoc |
||
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { | ||
if (recursionCount++ == 0) { | ||
// Note that the information about the addresses actually being accepted will be lost | ||
|
@@ -179,12 +180,15 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { | |
* EquivalentAddressGroup} addresses should be considered equivalent but may be flattened into a | ||
* single list if needed. | ||
* | ||
* <p>Implementations can choose to reject the given addresses by returning {@code false}. | ||
* <p>Implementations can choose to reject the given addresses by returning | ||
* {@code Status.UNAVAILABLE}. | ||
* | ||
* <p>Implementations should not modify the given {@code addresses}. | ||
* <p>Implementations should not modify the given {@code resolvedAddresses}. | ||
* | ||
* @param resolvedAddresses the resolved server addresses, attributes, and config. | ||
* @return {@code true} if the resolved addresses were accepted. {@code false} if rejected. | ||
* @return {@code Status.OK} if the resolved addresses were accepted. {@code Status.UNAVAILABLE} | ||
* if rejected. | ||
* | ||
* @since 1.49.0 | ||
*/ | ||
public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,8 +30,8 @@ public abstract class ForwardingLoadBalancer extends LoadBalancer { | |
protected abstract LoadBalancer delegate(); | ||
|
||
@Override | ||
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will need some more thought. This change would prevent extending classes from seeing the addresses, if they are only overriding handleResolvedAddresses(). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we mark this existing method as deprecated and introduce an override for acceptResolvedAddresses, please confirm There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would still break an implementation only implementing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Marking Something like,
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @shivaspeaks After removing delegate().acceptResolvedAddresses() from deprecated handleResolvedAddresses() method we are getting the PR checked failure with below compilation error warning: [InlineMeSuggester] This deprecated API looks inlineable. If you'd like the body of the API to be automatically inlined to its callers, please annotate it with @InlineMe. NOTE: the suggested fix makes the method final if it was not already. For more details please refer to the PR logs: https://github.com/grpc/grpc-java/actions/runs/13436677696/job/37540488347?pr=11623 Looking into this issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @shivaspeaks Attempting to use the @InlineMe annotation did not resolve the error as it mandates the method to be marked as final which is unsuitable, suppressing all warnings also failed to address the issue. Anything you would like to suggest here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Revert this method's implementation back to forwarding to |
||
delegate().handleResolvedAddresses(resolvedAddresses); | ||
public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { | ||
return delegate().acceptResolvedAddresses(resolvedAddresses); | ||
} | ||
|
||
@Override | ||
|
Uh oh!
There was an error while loading. Please reload this page.