-
Notifications
You must be signed in to change notification settings - Fork 105
feat(operations): Add WaitOperation API surface [gax-java] #1284
Changes from 5 commits
eef393e
b714927
7766dd5
989e540
d5c490f
e065dea
02dea8a
aba567a
badd77c
ad84f25
acfcbb5
a1d113f
a82b67e
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 |
---|---|---|
|
@@ -519,6 +519,61 @@ public final UnaryCallable<DeleteOperationRequest, Empty> deleteOperationCallabl | |
return stub.deleteOperationCallable(); | ||
} | ||
|
||
/** | ||
* Waits for the specified long-running operation until it is done or reaches at most a specified | ||
* timeout, returning the latest state. If the operation is already done, the latest state is | ||
* immediately returned. If the timeout specified is greater than the default HTTP/RPC timeout, | ||
* the HTTP/RPC timeout is used. If the server does not support this method, it returns | ||
* `google.rpc.Code.UNIMPLEMENTED`. Note that this method is on a best-effort basis. It may return | ||
* the latest state before the specified timeout (including immediately), meaning even an | ||
* immediate response is no guarantee that the operation is done. | ||
* | ||
* <p>Sample code: | ||
* | ||
* <pre><code> | ||
* try (OperationsClient operationsClient = OperationsClient.create()) { | ||
* String name = ""; | ||
* WaitOperationRequest request = WaitOperationRequest.newBuilder() | ||
* .setName(name) | ||
* .setTimeout(Duration.ofMillis(100)) | ||
* .build(); | ||
* Operation response = operationsClient.waitOperation(request); | ||
* } | ||
* </code></pre> | ||
* | ||
* @param request The request object containing all of the parameters for the API call. | ||
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. The --> the 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. Done. 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. need to push? 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. Pushed, thanks. 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. The --> the 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. Done. The rest of the comments in this file won't match, but that's fine. |
||
* @throws com.google.api.gax.rpc.ApiException if the remote call fails | ||
*/ | ||
public final Operation waitOperation(WaitOperationRequest request) { | ||
return waitOperationCallable().call(request); | ||
} | ||
|
||
/** | ||
* Waits for the specified long-running operation until it is done or reaches at most a specified | ||
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. "Waits for the specified long-running operation until it is" --> "Waits until the specified long-running operation is" 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. Same as above. |
||
* timeout, returning the latest state. If the operation is already done, the latest state is | ||
* immediately returned. If the timeout specified is greater than the default HTTP/RPC timeout, | ||
* the HTTP/RPC timeout is used. If the server does not support this method, it returns | ||
* `google.rpc.Code.UNIMPLEMENTED`. Note that this method is on a best-effort basis. It may return | ||
* the latest state before the specified timeout (including immediately), meaning even an | ||
* immediate response is no guarantee that the operation is done. | ||
* | ||
* <p>Sample code: | ||
* | ||
* <pre><code> | ||
* try (OperationsClient operationsClient = OperationsClient.create()) { | ||
* String name = ""; | ||
* WaitOperationRequest request = WaitOperationRequest.newBuilder() | ||
* .setName(name) | ||
* .setTimeout(Duration.ofMillis(100)) | ||
* .build(); | ||
* ApiFuture<Operation> future = operationsClient.waitOperationCallable().futureCall(request); | ||
* } | ||
* </code></pre> | ||
*/ | ||
public final UnaryCallable<WaitOperationRequest, Operation> waitOperationCallable() { | ||
return stub.waitOperationCallable(); | ||
} | ||
|
||
@Override | ||
public final void close() { | ||
stub.close(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,6 +72,11 @@ public UnaryCallSettings<DeleteOperationRequest, Empty> deleteOperationSettings( | |
return ((OperationsStubSettings) getStubSettings()).deleteOperationSettings(); | ||
} | ||
|
||
/** Returns the object with the settings used for calls to waitOperation. */ | ||
public UnaryCallSettings<WaitOperationRequest, Operation> waitOperationSettings() { | ||
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 looks testable 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 is a direct import of the generated GAPIC surface for the Wait* portion of the LRO proto, so none of the 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. I'm trying to understand whether you're proposing re-generating code in the future. That's not longterm maintainable. There are two paths that can work. Either the code is generated every time when the project is compiled, with the generated code never checked into the repo (the protobuf approach) or it's generated once, checked into the repo, and then manually edited for ever after. Anything in between makes maintenance extremely difficult. Which path is this PR proposing? 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. Ping. This is a critical 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. This PR (and others in this repo) only updates the LRO client with the auto-generated section for Wait, and does not propose a particular path. While the long-term path is not clear yet, keeping this client as consistent as possible with the generated code will make debugging less error prone and make updates easier, while we figure this out. 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. Autogenerated or no., I still think a test helps here. If the autogenerated code chages in a way that causes the test to fail, the test will make that apparent. 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. I've filed #1299 to track this, since adding tests for this method is essentially equivalent to setting up tests for all the settings getters, which is beyond the scope of this PR. This is a great point, and I'll look into whether we can add such autogenerated tests to gapic-generator-java. |
||
return ((OperationsStubSettings) getStubSettings()).waitOperationSettings(); | ||
} | ||
|
||
public static final OperationsSettings create(OperationsStubSettings stub) throws IOException { | ||
return new OperationsSettings.Builder(stub.toBuilder()).build(); | ||
} | ||
|
@@ -170,6 +175,11 @@ public UnaryCallSettings.Builder<DeleteOperationRequest, Empty> deleteOperationS | |
return getStubSettingsBuilder().deleteOperationSettings(); | ||
} | ||
|
||
/** Returns the builder for the settings used for calls to deleteOperation. */ | ||
public UnaryCallSettings.Builder<WaitOperationRequest, Operation> waitOperationSettings() { | ||
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. test 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. Ditto. 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. I still think there should be a test of this method. 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. As above - tracked in #1299, will investigate this feature for gapic-generator-java. |
||
return getStubSettingsBuilder().waitOperationSettings(); | ||
} | ||
|
||
@Override | ||
public OperationsSettings build() throws IOException { | ||
return new OperationsSettings(this); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,17 +37,21 @@ | |
import com.google.api.gax.grpc.GrpcCallSettings; | ||
import com.google.api.gax.grpc.GrpcStubCallableFactory; | ||
import com.google.api.gax.rpc.ClientContext; | ||
import com.google.api.gax.rpc.RequestParamsExtractor; | ||
import com.google.api.gax.rpc.UnaryCallable; | ||
import com.google.common.collect.ImmutableMap; | ||
import com.google.longrunning.CancelOperationRequest; | ||
import com.google.longrunning.DeleteOperationRequest; | ||
import com.google.longrunning.GetOperationRequest; | ||
import com.google.longrunning.ListOperationsRequest; | ||
import com.google.longrunning.ListOperationsResponse; | ||
import com.google.longrunning.Operation; | ||
import com.google.longrunning.WaitOperationRequest; | ||
import com.google.protobuf.Empty; | ||
import io.grpc.MethodDescriptor; | ||
import io.grpc.protobuf.ProtoUtils; | ||
import java.io.IOException; | ||
import java.util.Map; | ||
import java.util.concurrent.TimeUnit; | ||
import javax.annotation.Generated; | ||
|
||
|
@@ -97,6 +101,15 @@ public class GrpcOperationsStub extends OperationsStub { | |
ProtoUtils.marshaller(DeleteOperationRequest.getDefaultInstance())) | ||
.setResponseMarshaller(ProtoUtils.marshaller(Empty.getDefaultInstance())) | ||
.build(); | ||
private static final MethodDescriptor<WaitOperationRequest, Operation> | ||
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. A question about all of the changes in this PR: are these chagnes were added manually or generated, or some sort of "generated and then manually postprocessed"? |
||
waitOperationMethodDescriptor = | ||
MethodDescriptor.<WaitOperationRequest, Operation>newBuilder() | ||
.setType(MethodDescriptor.MethodType.UNARY) | ||
.setFullMethodName("google.longrunning.Operations/WaitOperation") | ||
.setRequestMarshaller( | ||
ProtoUtils.marshaller(WaitOperationRequest.getDefaultInstance())) | ||
.setResponseMarshaller(ProtoUtils.marshaller(Operation.getDefaultInstance())) | ||
.build(); | ||
|
||
private final BackgroundResource backgroundResources; | ||
|
||
|
@@ -106,6 +119,7 @@ public class GrpcOperationsStub extends OperationsStub { | |
listOperationsPagedCallable; | ||
private final UnaryCallable<CancelOperationRequest, Empty> cancelOperationCallable; | ||
private final UnaryCallable<DeleteOperationRequest, Empty> deleteOperationCallable; | ||
private final UnaryCallable<WaitOperationRequest, Operation> waitOperationCallable; | ||
|
||
private final GrpcStubCallableFactory callableFactory; | ||
|
||
|
@@ -149,19 +163,68 @@ protected GrpcOperationsStub( | |
GrpcCallSettings<GetOperationRequest, Operation> getOperationTransportSettings = | ||
GrpcCallSettings.<GetOperationRequest, Operation>newBuilder() | ||
.setMethodDescriptor(getOperationMethodDescriptor) | ||
.setParamsExtractor( | ||
new RequestParamsExtractor<GetOperationRequest>() { | ||
@Override | ||
public Map<String, String> extract(GetOperationRequest request) { | ||
ImmutableMap.Builder<String, String> params = ImmutableMap.builder(); | ||
params.put("name", String.valueOf(request.getName())); | ||
return params.build(); | ||
} | ||
}) | ||
.build(); | ||
GrpcCallSettings<ListOperationsRequest, ListOperationsResponse> | ||
listOperationsTransportSettings = | ||
GrpcCallSettings.<ListOperationsRequest, ListOperationsResponse>newBuilder() | ||
.setMethodDescriptor(listOperationsMethodDescriptor) | ||
.setParamsExtractor( | ||
new RequestParamsExtractor<ListOperationsRequest>() { | ||
@Override | ||
public Map<String, String> extract(ListOperationsRequest request) { | ||
ImmutableMap.Builder<String, String> params = ImmutableMap.builder(); | ||
params.put("name", String.valueOf(request.getName())); | ||
return params.build(); | ||
} | ||
}) | ||
.build(); | ||
GrpcCallSettings<CancelOperationRequest, Empty> cancelOperationTransportSettings = | ||
GrpcCallSettings.<CancelOperationRequest, Empty>newBuilder() | ||
.setMethodDescriptor(cancelOperationMethodDescriptor) | ||
.setParamsExtractor( | ||
new RequestParamsExtractor<CancelOperationRequest>() { | ||
@Override | ||
public Map<String, String> extract(CancelOperationRequest request) { | ||
ImmutableMap.Builder<String, String> params = ImmutableMap.builder(); | ||
params.put("name", String.valueOf(request.getName())); | ||
return params.build(); | ||
} | ||
}) | ||
.build(); | ||
GrpcCallSettings<DeleteOperationRequest, Empty> deleteOperationTransportSettings = | ||
GrpcCallSettings.<DeleteOperationRequest, Empty>newBuilder() | ||
.setMethodDescriptor(deleteOperationMethodDescriptor) | ||
.setParamsExtractor( | ||
new RequestParamsExtractor<DeleteOperationRequest>() { | ||
@Override | ||
public Map<String, String> extract(DeleteOperationRequest request) { | ||
ImmutableMap.Builder<String, String> params = ImmutableMap.builder(); | ||
params.put("name", String.valueOf(request.getName())); | ||
return params.build(); | ||
} | ||
}) | ||
.build(); | ||
GrpcCallSettings<WaitOperationRequest, Operation> waitOperationTransportSettings = | ||
GrpcCallSettings.<WaitOperationRequest, Operation>newBuilder() | ||
.setMethodDescriptor(waitOperationMethodDescriptor) | ||
.setParamsExtractor( | ||
new RequestParamsExtractor<WaitOperationRequest>() { | ||
@Override | ||
public Map<String, String> extract(WaitOperationRequest request) { | ||
ImmutableMap.Builder<String, String> params = ImmutableMap.builder(); | ||
params.put("name", String.valueOf(request.getName())); | ||
return params.build(); | ||
} | ||
}) | ||
.build(); | ||
|
||
this.getOperationCallable = | ||
|
@@ -179,6 +242,9 @@ protected GrpcOperationsStub( | |
this.deleteOperationCallable = | ||
callableFactory.createUnaryCallable( | ||
deleteOperationTransportSettings, settings.deleteOperationSettings(), clientContext); | ||
this.waitOperationCallable = | ||
callableFactory.createUnaryCallable( | ||
waitOperationTransportSettings, settings.waitOperationSettings(), clientContext); | ||
|
||
backgroundResources = new BackgroundResourceAggregation(clientContext.getBackgroundResources()); | ||
} | ||
|
@@ -204,6 +270,10 @@ public UnaryCallable<DeleteOperationRequest, Empty> deleteOperationCallable() { | |
return deleteOperationCallable; | ||
} | ||
|
||
public UnaryCallable<WaitOperationRequest, Operation> waitOperationCallable() { | ||
return waitOperationCallable; | ||
} | ||
|
||
@Override | ||
public final void close() { | ||
shutdown(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,7 @@ | |
import com.google.longrunning.ListOperationsRequest; | ||
import com.google.longrunning.ListOperationsResponse; | ||
import com.google.longrunning.Operation; | ||
import com.google.longrunning.WaitOperationRequest; | ||
import com.google.protobuf.Empty; | ||
import javax.annotation.Generated; | ||
|
||
|
@@ -74,6 +75,10 @@ public UnaryCallable<DeleteOperationRequest, Empty> deleteOperationCallable() { | |
throw new UnsupportedOperationException("Not implemented: deleteOperationCallable()"); | ||
} | ||
|
||
public UnaryCallable<WaitOperationRequest, Operation> waitOperationCallable() { | ||
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.
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. I don't think there's a corresponding method in 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. Also, I'll be adding this to the GAPIC generation, so this will be consistent with a future generated surface. |
||
throw new UnsupportedOperationException("Not implemented: waitOperationCallable()"); | ||
} | ||
|
||
@Override | ||
public abstract void close(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,6 +61,7 @@ | |
import com.google.longrunning.ListOperationsRequest; | ||
import com.google.longrunning.ListOperationsResponse; | ||
import com.google.longrunning.Operation; | ||
import com.google.longrunning.WaitOperationRequest; | ||
import com.google.protobuf.Empty; | ||
import java.io.IOException; | ||
import javax.annotation.Generated; | ||
|
@@ -78,6 +79,7 @@ public class OperationsStubSettings extends StubSettings<OperationsStubSettings> | |
listOperationsSettings; | ||
private final UnaryCallSettings<CancelOperationRequest, Empty> cancelOperationSettings; | ||
private final UnaryCallSettings<DeleteOperationRequest, Empty> deleteOperationSettings; | ||
private final UnaryCallSettings<WaitOperationRequest, Operation> waitOperationSettings; | ||
|
||
/** Returns the object with the settings used for calls to getOperation. */ | ||
public UnaryCallSettings<GetOperationRequest, Operation> getOperationSettings() { | ||
|
@@ -101,6 +103,11 @@ public UnaryCallSettings<DeleteOperationRequest, Empty> deleteOperationSettings( | |
return deleteOperationSettings; | ||
} | ||
|
||
/** Returns the object with the settings used for calls to waitOperation. */ | ||
public UnaryCallSettings<WaitOperationRequest, Operation> waitOperationSettings() { | ||
return waitOperationSettings; | ||
} | ||
|
||
@BetaApi("A restructuring of stub classes is planned, so this may break in the future") | ||
public OperationsStub createStub() throws IOException { | ||
if (getTransportChannelProvider() | ||
|
@@ -154,6 +161,7 @@ protected OperationsStubSettings(Builder settingsBuilder) throws IOException { | |
listOperationsSettings = settingsBuilder.listOperationsSettings().build(); | ||
cancelOperationSettings = settingsBuilder.cancelOperationSettings().build(); | ||
deleteOperationSettings = settingsBuilder.deleteOperationSettings().build(); | ||
waitOperationSettings = settingsBuilder.waitOperationSettings().build(); | ||
} | ||
|
||
private static final PagedListDescriptor<ListOperationsRequest, ListOperationsResponse, Operation> | ||
|
@@ -218,6 +226,7 @@ public static class Builder extends StubSettings.Builder<OperationsStubSettings, | |
listOperationsSettings; | ||
private final UnaryCallSettings.Builder<CancelOperationRequest, Empty> cancelOperationSettings; | ||
private final UnaryCallSettings.Builder<DeleteOperationRequest, Empty> deleteOperationSettings; | ||
private final UnaryCallSettings.Builder<WaitOperationRequest, Operation> waitOperationSettings; | ||
|
||
private static final ImmutableMap<String, ImmutableSet<StatusCode.Code>> | ||
RETRYABLE_CODE_DEFINITIONS; | ||
|
@@ -268,6 +277,8 @@ protected Builder(ClientContext clientContext) { | |
|
||
deleteOperationSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); | ||
|
||
waitOperationSettings = UnaryCallSettings.newUnaryCallSettingsBuilder(); | ||
|
||
unaryMethodSettingsBuilders = | ||
ImmutableList.<UnaryCallSettings.Builder<?, ?>>of( | ||
getOperationSettings, | ||
|
@@ -305,6 +316,11 @@ private static Builder initDefaults(Builder builder) { | |
.setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("idempotent")) | ||
.setRetrySettings(RETRY_PARAM_DEFINITIONS.get("default")); | ||
|
||
builder | ||
.waitOperationSettings() | ||
.setRetryableCodes(RETRYABLE_CODE_DEFINITIONS.get("idempotent")) | ||
.setRetrySettings(RETRY_PARAM_DEFINITIONS.get("default")); | ||
|
||
return builder; | ||
} | ||
|
||
|
@@ -315,13 +331,15 @@ protected Builder(OperationsStubSettings settings) { | |
listOperationsSettings = settings.listOperationsSettings.toBuilder(); | ||
cancelOperationSettings = settings.cancelOperationSettings.toBuilder(); | ||
deleteOperationSettings = settings.deleteOperationSettings.toBuilder(); | ||
waitOperationSettings = settings.waitOperationSettings.toBuilder(); | ||
|
||
unaryMethodSettingsBuilders = | ||
ImmutableList.<UnaryCallSettings.Builder<?, ?>>of( | ||
getOperationSettings, | ||
listOperationsSettings, | ||
cancelOperationSettings, | ||
deleteOperationSettings); | ||
deleteOperationSettings, | ||
waitOperationSettings); | ||
} | ||
|
||
/** | ||
|
@@ -361,6 +379,11 @@ public UnaryCallSettings.Builder<DeleteOperationRequest, Empty> deleteOperationS | |
return deleteOperationSettings; | ||
} | ||
|
||
/** Returns the builder for the settings used for calls to waitOperation. */ | ||
public UnaryCallSettings.Builder<WaitOperationRequest, Operation> waitOperationSettings() { | ||
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. test 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. Same as above. |
||
return waitOperationSettings; | ||
} | ||
|
||
@Override | ||
public OperationsStubSettings build() throws IOException { | ||
return new OperationsStubSettings(this); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -130,4 +130,19 @@ public void cancelOperation( | |
responseObserver.onError(new IllegalArgumentException("Unrecognized response type")); | ||
} | ||
} | ||
|
||
@Override | ||
public void waitOperation( | ||
WaitOperationRequest request, StreamObserver<Operation> responseObserver) { | ||
Object response = responses.remove(); | ||
if (response instanceof Operation) { | ||
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 is very strange. The response can have multiple types? 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. Copied verbatim from the standard generated GAPIC library for the LRO proto, as are the methods above. |
||
requests.add(request); | ||
responseObserver.onNext((Operation) response); | ||
responseObserver.onCompleted(); | ||
} else if (response instanceof Exception) { | ||
responseObserver.onError((Exception) response); | ||
} else { | ||
responseObserver.onError(new IllegalArgumentException("Unrecognized response type")); | ||
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. include the type the in the message here to ease debugging 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. Done, will add to the microgenerator as well. |
||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Waits for the specified long-running operation until it is" --> "Waits until the specified long-running operation is"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is meant to reflect the RPC's comment verbatim. If I change it here, we'll need to update the protos as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this PR checking in generated code? I'm not sure we need to reflect the RPC's comment verbatim (though, yes, it should be changed there too.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The delta here is nearly identical to the generated code, so yes. I'm adding it in this way (as opposed to copy-pasting the new GAPIC client entirely) to preserve existing manual changes and ensure that dependencies won't break.
Updated the comment.