-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add Rekognition support. #4957
Add Rekognition support. #4957
Conversation
Hi @bblommers following on from #4955 - I have had a crack at adding Rekognition support to moto. I am sure that I have got a few bits wrong/ missed something, as this is the first time working in this codebase. If someone could review the code as is and I can then amend. Thank you. |
cd66e2a
to
ea97597
Compare
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.
Hi @AlanDonohoe, thanks for the PR! Please see my comments.
If you want to run the linter locally, to verify it passes, you can run make lint
.
1af0a22
to
c72a44b
Compare
OK, @bblommers I have made a number of amendments based on your feedback and the code is ready for a re-review please. Thank you. |
LGTM so far @AlanDonohoe - happy to merge once the linter is happy (and assuming the build passes, obviously) |
At the moment just adding support for start_text_detection and get_text_detection. refs: https://docs.aws.amazon.com/rekognition/latest/dg/what-is.html https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/rekognition.html
c72a44b
to
f37bd67
Compare
OK cool... There was one linter issue, which I have since resolved. Have pushed that up now, so once all the linter checks and tests are good, you'll be happy to merge in! And I can use moto's new Rekognition support back in my original project! I'm going to resolve all the conversations up to this point so it's clear what amendments have been subsequently fixed up. |
At the moment just adding support for start_text_detection and get_text_detection. refs: https://docs.aws.amazon.com/rekognition/latest/dg/what-is.html https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/rekognition.html
f37bd67
to
756e690
Compare
@AlanDonohoe There are still some test failures, but they should be resolved after you merge this PR that I raised: LiniusAustPty#1 |
Fix Rekognition build errors
@bblommers Thanks for that... I have merged in. Do you want me to do some rebasing now on this branch (or are the commits OK as is?) |
No, that's not necessary, I'll just squash and merge. Thank you for all your work on this @AlanDonohoe! |
This is now part of moto >= 3.1.4.dev3 |
Amazing. V happy this will be in the next release. Thanks for all the feedback and assistance @bblommers |
…_search. Follows on from this work: getmoto#4957 refs: https://docs.aws.amazon.com/rekognition/latest/dg/what-is.html https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/rekognition.html More specifically: https://docs.aws.amazon.com/rekognition/latest/APIReference/API_GetFaceSearch.html https://docs.aws.amazon.com/rekognition/latest/APIReference/API_StartFaceSearch.html Related issue: getmoto#4071
…_search. Follows on from this work: getmoto#4957 refs: https://docs.aws.amazon.com/rekognition/latest/dg/what-is.html https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/rekognition.html More specifically: https://docs.aws.amazon.com/rekognition/latest/APIReference/API_GetFaceSearch.html https://docs.aws.amazon.com/rekognition/latest/APIReference/API_StartFaceSearch.html Related issue: getmoto#4071
Description
Adds Rekognition support.
Background Context
At the moment just adding support for start_text_detection and get_text_detection.
refs:
https://docs.aws.amazon.com/rekognition/latest/dg/what-is.html
https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/rekognition.html
#4071