-
Notifications
You must be signed in to change notification settings - Fork 0
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
Transcribe #28
Transcribe #28
Conversation
and handling the transcription response. | ||
*/ | ||
|
||
public class TranscribeMedicalStreamingDemoApp { |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
This class references 43 other classes. By comparison, 98% of the classes in the CodeGuru reference dataset reference fewer. This indicates that this class is highly coupled with other classes. A class that is highly coupled with other classes is difficult to understand and its behavior might change unexpectedly when one of its referenced classes is updated. High coupling could also increase the integration test complexity, maintenance cost and technical debt. We recommend that you simplify this class or break it into multiple classes.
""" | ||
try: | ||
# Get list of all objects in the bucket. | ||
list_response = self.s3_client.list_objects_v2(Bucket=bucket_name) |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
The API method list_objects_v2
returns paginated results instead of all results. Consider using the pagination API or checking one of the following keys in the response to verify that all results were returned: ContinuationToken
, IsTruncated
, NextContinuationToken
, NextToken
.
"CopySourceIfUnmodifiedSince", | ||
] | ||
|
||
yesterday_date = datetime.datetime.utcnow() - datetime.timedelta(days=1) |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Naive datetime objects are treated by many datetime methods as local times, We recommend that you create aware timestamps with the now
or fromtimestamp
functions using the tzinfo
to attribute to represent times in UTC. Do not use the datetime.utcnow() because it returns only a naive
datetime object.
System.exit(0); | ||
} | ||
|
||
TargetDataLine line = (TargetDataLine) AudioSystem.getLine(info); |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Problem
This line of code might contain a resource leak. Resource leaks can cause your system to slow down or crash.
Fix
Consider closing the following resource: line. Currently, there are execution paths that do not contain closure statements, for example, when TargetDataLine.open throws an exception. Ensure line is closed in a try-finally block or declared in a try-with-resources block.
More info
View resource management guidelines at oracle.com (external link).
new AudioStreamPublisher(getStreamFromMic()), | ||
getMedicalResponseHandler()); | ||
|
||
result.get(); |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
It looks like you are using CompletableFuture.get()
or Future.get()
without setting its timeout parameter. Be sure to set timeout values or handle timeout exceptions to prevent threads from waiting for a process to end. This can lead to downtime for your customers. For more information, see CompletableFuture and Future documentation.
Suggested remediation:
Set timeout parameters for CompletableFuture.get()
or Future.get()
.Exceptions, such as ExecutionException
, InterruptedException
and TimeoutException
are caught and errors are logged.
@@ -54,3 +54,14 @@
- result.get();
+ try {
+ result.get(100, TimeUnit.MILLISECONDS);
+ } catch (InterruptedException ie) {
+ log.error("Thread interrupted.", ie);
+ throw new InterruptedException(ie.getMessage());
+ } catch (ExecutionException xe) {
+ log.error("The task aborted when attempting to retrieve its result.", xe);
+ throw new ExecutionException(xe.getMessage());
+ } catch (TimeoutException te) {
+ log.error("Future is not done when timeout.", te);
+ throw new TimeoutException(te.getMessage());
+ }
client.close();
gov2/s3/actions/bucket_basics.go
Outdated
} | ||
} else if len(output.Errors) > 0 { | ||
for _, outErr := range output.Errors { | ||
log.Printf("%s: %s\n", *outErr.Key, *outErr.Message) |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Attempting to access memory locations that are null or uninitialized can lead to nil pointer dereferences and crashes. Always check that pointers are not nil before dereferencing them. Validate return values from functions that provide pointers, and handle potential nil values appropriately. Adding nil checks will prevent unexpected errors and issues when accessing memory.
Suggested remediation:
We recommend the following changes to mitigate the issue.
@@ -365,5 +365,10 @@
} else if len(output.Errors) > 0 {
for _, outErr := range output.Errors {
- log.Printf("%s: %s\n", *outErr.Key, *outErr.Message)
+ if outErr != nil {
+ log.Printf("%s: %s\n", *outErr.Key, *outErr.Message)
+}
+else {
+ log.Fatal("Error: Attempting to dereference a nil pointer")
+}
}
err = fmt.Errorf("%s", *output.Errors[0].Message)
err = s3.NewObjectNotExistsWaiter(actor.S3Client).Wait( | ||
ctx, &s3.HeadObjectInput{Bucket: aws.String(bucket), Key: delObjs.Key}, time.Minute) | ||
if err != nil { | ||
log.Printf("Failed attempt to wait for object %s to be deleted.\n", *delObjs.Key) |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Attempting to access memory locations that are null or uninitialized can lead to nil pointer dereferences and crashes. Always check that pointers are not nil before dereferencing them. Validate return values from functions that provide pointers, and handle potential nil values appropriately. Adding nil checks will prevent unexpected errors and issues when accessing memory.
gov2/s3/actions/bucket_basics.go
Outdated
for _, outErr := range output.Errors { | ||
log.Printf("%s: %s\n", *outErr.Key, *outErr.Message) | ||
} | ||
err = fmt.Errorf("%s", *output.Errors[0].Message) |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Attempting to access memory locations that are null or uninitialized can lead to nil pointer dereferences and crashes. Always check that pointers are not nil before dereferencing them. Validate return values from functions that provide pointers, and handle potential nil values appropriately. Adding nil checks will prevent unexpected errors and issues when accessing memory.
gov2/s3/actions/bucket_basics.go
Outdated
err = s3.NewObjectNotExistsWaiter(basics.S3Client).Wait( | ||
ctx, &s3.HeadObjectInput{Bucket: aws.String(bucketName), Key: delObjs.Key}, time.Minute) | ||
if err != nil { | ||
log.Printf("Failed attempt to wait for object %s to be deleted.\n", *delObjs.Key) |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Attempting to access memory locations that are null or uninitialized can lead to nil pointer dereferences and crashes. Always check that pointers are not nil before dereferencing them. Validate return values from functions that provide pointers, and handle potential nil values appropriately. Adding nil checks will prevent unexpected errors and issues when accessing memory.
if err != nil { | ||
log.Panicf("Couldn't create destination archive %v. Here's why: %v\n", destinationFile, err) | ||
} | ||
sourceBody, err := os.ReadFile(fmt.Sprintf("%v/%v", helper.HandlerPath, sourceFile)) |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
User input controlling server file access enables path traversal, allowing access to unintended files. Sanitize and validate file paths before usage. Recommend allowlisting permitted paths and filenames. Never concatenate or join user input into filesystem paths. Input validation prevents malicious actors from accessing sensitive files.
d07e409
to
a8acac3
Compare
and handling the transcription response. | ||
*/ | ||
|
||
public class TranscribeMedicalStreamingDemoApp { |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
This class references 42 other classes. By comparison, 98% of the classes in the CodeGuru reference dataset reference fewer. This indicates that this class is highly coupled with other classes. A class that is highly coupled with other classes is difficult to understand and its behavior might change unexpectedly when one of its referenced classes is updated. High coupling could also increase the integration test complexity, maintenance cost and technical debt. We recommend that you simplify this class or break it into multiple classes.
new AudioStreamPublisher(getStreamFromMic()), | ||
getMedicalResponseHandler()); | ||
|
||
result.get(); |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
It looks like you are using CompletableFuture.get()
or Future.get()
without setting its timeout parameter. Be sure to set timeout values or handle timeout exceptions to prevent threads from waiting for a process to end. This can lead to downtime for your customers. For more information, see CompletableFuture and Future documentation.
Suggested remediation:
Set timeout parameters for CompletableFuture.get()
or Future.get()
.Exceptions, such as ExecutionException
, InterruptedException
and TimeoutException
are caught and errors are logged.
@@ -52,3 +52,14 @@
- result.get();
+ try {
+ result.get(100, TimeUnit.MILLISECONDS);
+ } catch (InterruptedException ie) {
+ log.error("Thread interrupted.", ie);
+ throw new InterruptedException(ie.getMessage());
+ } catch (ExecutionException xe) {
+ log.error("The task aborted when attempting to retrieve its result.", xe);
+ throw new ExecutionException(xe.getMessage());
+ } catch (TimeoutException te) {
+ log.error("Future is not done when timeout.", te);
+ throw new TimeoutException(te.getMessage());
+ }
client.close();
import java.util.concurrent.atomic.AtomicLong; | ||
|
||
// snippet-start:[transcribe.java-streaming-demo] | ||
public class TranscribeStreamingDemoApp { |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
This class references 48 other classes. By comparison, 98% of the classes in the CodeGuru reference dataset reference fewer. This indicates that this class is highly coupled with other classes. A class that is highly coupled with other classes is difficult to understand and its behavior might change unexpectedly when one of its referenced classes is updated. High coupling could also increase the integration test complexity, maintenance cost and technical debt. We recommend that you simplify this class or break it into multiple classes.
and handling the transcription response. | ||
*/ | ||
|
||
public class TranscribeMedicalStreamingDemoApp { |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
This class references 42 other classes. By comparison, 98% of the classes in the CodeGuru reference dataset reference fewer. This indicates that this class is highly coupled with other classes. A class that is highly coupled with other classes is difficult to understand and its behavior might change unexpectedly when one of its referenced classes is updated. High coupling could also increase the integration test complexity, maintenance cost and technical debt. We recommend that you simplify this class or break it into multiple classes.
System.out.println("Line not supported"); | ||
System.exit(0); | ||
} | ||
|
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Problem
This line of code might contain a resource leak. Resource leaks can cause your system to slow down or crash.
Fix
Consider closing the following resource: line. Currently, there are execution paths that do not contain closure statements, for example, when TargetDataLine.open throws an exception. Ensure line is closed in a try-finally block or declared in a try-with-resources block.
More info
View resource management guidelines at oracle.com (external link).
AudioFormat format = new AudioFormat(16000, 16, 1, true, false); | ||
DataLine.Info datalineInfo = new DataLine.Info(TargetDataLine.class, format); | ||
|
||
TargetDataLine dataLine = (TargetDataLine) AudioSystem.getLine(datalineInfo); |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Problem
This line of code contains a resource that might not be closed properly. This can cause a resource leak that slows down or crashes your system.
Fix
Consider closing the following resource: dataLine. The resource is referenced at lines 73, 74. The resource is returned at line 75. There are other execution paths that don't close the resource or return it, for example, when TargetDataLine.open throws an exception. To prevent this resource leak, close dataLine along these other paths before you exit this method.
More info
View resource management guidelines at oracle.com (external link).
throw new LineUnavailableException("The audio system microphone line is not supported."); | ||
} | ||
|
||
TargetDataLine line = (TargetDataLine) AudioSystem.getLine(info); |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Problem
This line of code might contain a resource leak. Resource leaks can cause your system to slow down or crash.
Fix
Consider closing the following resource: line. Currently, there are execution paths that do not contain closure statements, for example, when TargetDataLine.open throws an exception. Ensure line is closed in a try-finally block or declared in a try-with-resources block.
More info
View resource management guidelines at oracle.com (external link).
* Create Amazon Transcribe streaming retry client. | ||
*/ | ||
|
||
TranscribeStreamingRetryClient client = new TranscribeStreamingRetryClient( |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Problem
Analysis of this package determined that this line of code contains a resource that might not have closed properly. A resource leak can slow down or crash your system.
Fix
Consider closing the following resource in a try-finally block: client. The resource is referenced at line 51. The resource is closed at line 72. There are other execution paths that do not contain closure statements, for example, when FileInputStream constructor throws an exception.
More info
View resource management guidelines at oracle.com (external link).
new AudioStreamPublisher(getStreamFromFile(file)), | ||
getResponseHandler()); | ||
|
||
result.get(); |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
It looks like you are using CompletableFuture.get()
or Future.get()
without setting its timeout parameter. Be sure to set timeout values or handle timeout exceptions to prevent threads from waiting for a process to end. This can lead to downtime for your customers. For more information, see CompletableFuture and Future documentation.
Suggested remediation:
Set timeout parameters for CompletableFuture.get()
or Future.get()
.Exceptions, such as ExecutionException
, InterruptedException
and TimeoutException
are caught and errors are logged.
@@ -59,3 +59,14 @@
- result.get();
+ try {
+ result.get(100, TimeUnit.MILLISECONDS);
+ } catch (InterruptedException ie) {
+ log.error("Thread interrupted.", ie);
+ throw new InterruptedException(ie.getMessage());
+ } catch (ExecutionException xe) {
+ log.error("The task aborted when attempting to retrieve its result.", xe);
+ throw new ExecutionException(xe.getMessage());
+ } catch (TimeoutException te) {
+ log.error("Future is not done when timeout.", te);
+ throw new TimeoutException(te.getMessage());
+ }
client.close();
CompletableFuture<Void> result = client.startStreamTranscription(getRequest(16_000), | ||
new AudioStreamPublisher(getStreamFromMic()), | ||
getResponseHandler()); | ||
|
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
It looks like you are using CompletableFuture.get()
or Future.get()
without setting its timeout parameter. Be sure to set timeout values or handle timeout exceptions to prevent threads from waiting for a process to end. This can lead to downtime for your customers. For more information, see CompletableFuture and Future documentation.
Suggested remediation:
Set timeout parameters for CompletableFuture.get()
or Future.get()
.Exceptions, such as ExecutionException
, InterruptedException
and TimeoutException
are caught and errors are logged.
@@ -42,3 +42,14 @@
- result.get();
+ try {
+ result.get(100, TimeUnit.MILLISECONDS);
+ } catch (InterruptedException ie) {
+ log.error("Thread interrupted.", ie);
+ throw new InterruptedException(ie.getMessage());
+ } catch (ExecutionException xe) {
+ log.error("The task aborted when attempting to retrieve its result.", xe);
+ throw new ExecutionException(xe.getMessage());
+ } catch (TimeoutException te) {
+ log.error("Future is not done when timeout.", te);
+ throw new TimeoutException(te.getMessage());
+ }
client.close();
|
||
/** | ||
* Synchronous wait for stream to close, and close client connection | ||
*/ |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
It looks like you are using CompletableFuture.get()
or Future.get()
without setting its timeout parameter. Be sure to set timeout values or handle timeout exceptions to prevent threads from waiting for a process to end. This can lead to downtime for your customers. For more information, see CompletableFuture and Future documentation.
Suggested remediation:
Set timeout parameters for CompletableFuture.get()
or Future.get()
.Exceptions, such as ExecutionException
, InterruptedException
and TimeoutException
are caught and errors are logged.
@@ -70,3 +70,17 @@
*/
- result.get();
+ try {
+ /**
+ * Synchronous wait for stream to close, and close client connection
+ */
+ result.get(100, TimeUnit.MILLISECONDS);
+ } catch (InterruptedException ie) {
+ log.error("Thread interrupted.", ie);
+ throw new InterruptedException(ie.getMessage());
+ } catch (ExecutionException xe) {
+ log.error("The task aborted when attempting to retrieve its result.", xe);
+ throw new ExecutionException(xe.getMessage());
+ } catch (TimeoutException te) {
+ log.error("Future is not done when timeout.", te);
+ throw new TimeoutException(te.getMessage());
+ }
client.close();
new AudioStreamPublisher(getStreamFromMic()), | ||
getMedicalResponseHandler()); | ||
|
||
result.get(); |
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.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
It looks like you are using CompletableFuture.get()
or Future.get()
without setting its timeout parameter. Be sure to set timeout values or handle timeout exceptions to prevent threads from waiting for a process to end. This can lead to downtime for your customers. For more information, see CompletableFuture and Future documentation.
Suggested remediation:
Set timeout parameters for CompletableFuture.get()
or Future.get()
.Exceptions, such as ExecutionException
, InterruptedException
and TimeoutException
are caught and errors are logged.
@@ -52,3 +52,14 @@
- result.get();
+ try {
+ result.get(100, TimeUnit.MILLISECONDS);
+ } catch (InterruptedException ie) {
+ log.error("Thread interrupted.", ie);
+ throw new InterruptedException(ie.getMessage());
+ } catch (ExecutionException xe) {
+ log.error("The task aborted when attempting to retrieve its result.", xe);
+ throw new ExecutionException(xe.getMessage());
+ } catch (TimeoutException te) {
+ log.error("Future is not done when timeout.", te);
+ throw new TimeoutException(te.getMessage());
+ }
client.close();
a28526f
to
97c75c6
Compare
97c75c6
to
c52ad86
Compare
This pull request...
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.