gemini-code-assist[bot] commented on code in PR #36119: URL: https://github.com/apache/beam/pull/36119#discussion_r2341207084
########## sdks/java/io/components/src/main/java/org/apache/beam/sdk/io/components/throttling/ThrottlingSignaler.java: ########## @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.beam.sdk.io.components.throttling; + +import org.apache.beam.sdk.metrics.Counter; +import org.apache.beam.sdk.metrics.Metrics; +/** + * The ThrottlingSignaler is a utility class for IOs to signal to the runner + * that a process is being throttled, preventing autoscaling. This is primarily + * used when making calls to a remote service where quotas and rate limiting + * are reasonable considerations. + */ +public class ThrottlingSignaler { + private Counter throttle_counter; + + public ThrottlingSignaler(String namespace) { + this.throttle_counter = Metrics.counter(namespace, Metrics.THROTTLE_TIME_COUNTER_NAME); + } + + public ThrottlingSignaler() { + ThrottlingSignaler(""); + } + + /** + * Signal that a transform has been throttled for an amount of time + * represented in milliseconds. + */ + public void signalThrottling(long milliseconds) { + throttle_counter.inc(milliseconds); + } Review Comment:  There are a couple of issues in this block: 1. **Critical**: The constructor call `ThrottlingSignaler("");` on line 36 is incorrect and will cause a compilation error. To delegate to another constructor in the same class, you must use `this("");`. 2. **Style**: The field `throttle_counter` on line 29 uses snake_case, which is not standard for Java. It should be renamed to `throttleCounter` to follow the camelCase convention. 3. **Improvement**: Since the counter field is only assigned in the constructor, it can be declared as `final`. This is a good practice for immutability and makes the class more robust. I've provided a code suggestion that addresses all these points. ```suggestion private final Counter throttleCounter; public ThrottlingSignaler(String namespace) { this.throttleCounter = Metrics.counter(namespace, Metrics.THROTTLE_TIME_COUNTER_NAME); } public ThrottlingSignaler() { this(""); } /** * Signal that a transform has been throttled for an amount of time * represented in milliseconds. */ public void signalThrottling(long milliseconds) { throttleCounter.inc(milliseconds); } ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
