-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Python PTransform wrapper for AWS SQS #34581
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
base: master
Are you sure you want to change the base?
Python PTransform wrapper for AWS SQS #34581
Conversation
fixes #34326 |
""" | ||
URN = "beam:schematransform:org.apache.beam:aws:sqs_read:v1" | ||
|
||
def __init__( |
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.
(question to @ahmedabu98) Shall we add this external transform to auto wrapper?
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.
Yeah this looks like something that can be automated using standard_expansion_services.yaml
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.
and the integration test below can dynamically generate and use the transform with ExternalTransformProvider
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.
quick question, can the integration test use the newly generated transform instead of the ExternalTransformProvider? given the dependencies the new Python wrapper should already be in place, is that right?
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.
Ok, I made the changes to make the IT use the ExternalTransformProvider. Thanks for for the pointers.
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.
I think it depends -- some test environments won't set up the SDK before testing, i.e. it won't run the code that generates the wrapper file. So it's safer to use ExternalTransformProvider
Thanks for making those changes
Assigning reviewers. If you would like to opt out of this review, comment R: @jrmccluskey for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
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.
Thanks for this change @prodriguezdefino, left some comments
I'm curious, how did you find the instructions? any feedback?
@@ -56,6 +56,31 @@ tasks.register("postCommitIT") { | |||
} | |||
} | |||
|
|||
tasks.register("xlangAwsIOIT") { |
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.
You can automate this part using our existing utils. Steps:
- Create a new pytest marker here
- Mark your new test in
xlang_sqsio_it_test.py
with pytest.mark. - Follow these instructions:
beam/sdks/python/test-suites/xlang/build.gradle
Lines 24 to 25 in 0536dc6
// To run tests that use another expansion service, create a new CrossLanguageTask with the // relevant fields as done here, then add it to `xlangTasks`.
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.
If you name the new task "awsCrossLanguage" for example, it will create a new gradle task ":sdks:python:test-suites:direct:py<version>:awsCrossLanguagePythonUsingJava"
that you can reference in sdks/python/test-suites/direct/build.gradle
Completes the work started on #34327 to expose the SQS read functionality to the Python SDK.
Added integration tests for the Direct Runner executed as part of a post commit suite. Tested execution of the new GitHub workflow in the forked repository, see link.
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.