X Tutup
Skip to content
This repository was archived by the owner on Mar 23, 2026. It is now read-only.

[AWS][Transcribe] Adding fix for validating Audio length#12450

Merged
k-a-il merged 9 commits intolocalstack:masterfrom
brunodmartins:fix-gh12423
Apr 16, 2025
Merged

[AWS][Transcribe] Adding fix for validating Audio length#12450
k-a-il merged 9 commits intolocalstack:masterfrom
brunodmartins:fix-gh12423

Conversation

@brunodmartins
Copy link
Contributor

@brunodmartins brunodmartins commented Mar 28, 2025 •
edited
Loading

Motivation

Closes #12423.

Changes

In case the audio file has more than 4 hours, it will accept the job execution, but fails upon trying to process.

Testing

  • Try to execute a transcribe job with an audio file with less than 4 hours, and it should accept and fulfill the execution
  • Try to execute a transcribe job with an audio file with equal or more than 4 hours, and it should fail the execution

@localstack-bot
Copy link
Contributor

localstack-bot commented Mar 28, 2025 •
edited
Loading

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

localstack-bot
localstack-bot reviewed Mar 28, 2025
View reviewed changes
Copy link
Contributor

@localstack-bot localstack-bot left a comment

Choose a reason for hiding this comment

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

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

@brunodmartins brunodmartins mentioned this pull request Mar 28, 2025
Closed
1 task
@brunodmartins
Copy link
Contributor Author

brunodmartins commented Mar 28, 2025

I have read the CLA Document and I hereby sign the CLA

localstack-bot added a commit that referenced this pull request Mar 28, 2025
@localstack-bot
@brunodmartins has signed the CLA in #12450
ed22361
@brunodmartins brunodmartins marked this pull request as ready for review March 28, 2025 23:07
@brunodmartins brunodmartins changed the title Adding fix for validating audit length [AWS][Transcribe] Adding fix for validating Audio length Mar 29, 2025
@sannya-singal sannya-singal added the semver: patch Non-breaking changes which can be included in patch releases label Mar 31, 2025
sannya-singal
sannya-singal previously requested changes Mar 31, 2025
View reviewed changes
Copy link
Contributor

@sannya-singal sannya-singal left a comment

Choose a reason for hiding this comment

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

Thanks @brunodmartins for working on this issue 🚀 I have suggested some changes and proposed to add an AWS validated test. Let me know if you would like more assistance on this 🙂 Happy to help!

@sannya-singal sannya-singal requested a review from k-a-il April 11, 2025 08:57
sannya-singal
sannya-singal reviewed Apr 11, 2025
View reviewed changes
@brunodmartins
Copy link
Contributor Author

brunodmartins commented Apr 12, 2025

@sannya-singal Thanks for your help! I could write down the required snapshots tests and validate it against AWS! Could you review it again?

I detected a problem on running the tests on my Macbook, which failed to exec the required ffmpeg programs to validate the transcribe tests. I succeeded it running it on an old Linux machine. I will investigate it further and open an Issue and a PR =)

k-a-il
k-a-il suggested changes Apr 15, 2025
View reviewed changes
Copy link
Collaborator

@k-a-il k-a-il left a comment

Choose a reason for hiding this comment

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

Hi @brunodmartins, I’m handling this PR for @sannya-singal while she is away.
Thank you for implementing this and addressing @sannya-singal 's comments 🚀 🎉 I have added two smaller comments. Let me know if any help is needed

@brunodmartins brunodmartins requested a review from k-a-il April 15, 2025 19:44
@k-a-il k-a-il requested review from bentsku and removed request for sannya-singal April 16, 2025 15:11
@bentsku bentsku dismissed sannya-singal’s stale review April 16, 2025 15:13

The changes Sannya requested have been addressed, and while she is OOO we can merge the PR

bentsku
bentsku approved these changes Apr 16, 2025
View reviewed changes
Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for adding this parity improvement while taking advantage of the fact we already had that data available to us. 😄 welcome to the contributors of LocalStack!

k-a-il
k-a-il approved these changes Apr 16, 2025
View reviewed changes
Copy link
Collaborator

@k-a-il k-a-il left a comment

Choose a reason for hiding this comment

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

@brunodmartins, great work on the PR! we are ready to merge 🚀

@k-a-il k-a-il merged commit ef8845d into localstack:master Apr 16, 2025
19 checks passed
@brunodmartins
Copy link
Contributor Author

brunodmartins commented Apr 16, 2025

Thanks for the help! I will look forward on another contributions on this repo =)

@yashuatla yashuatla mentioned this pull request Jun 23, 2025
Open
@purcell purcell mentioned this pull request Feb 9, 2026
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

4 more reviewers

@sannya-singal sannya-singal sannya-singal left review comments

@localstack-bot localstack-bot localstack-bot left review comments

@bentsku bentsku bentsku approved these changes

@k-a-il k-a-il k-a-il approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: TranscribeService should fail for audio sizes exceeding 4 hours.

5 participants

@brunodmartins @localstack-bot @sannya-singal @bentsku @k-a-il
X Tutup