Skip to content
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

Improve AWS service integration support (pattern) #74

Closed
wants to merge 2 commits into from

Conversation

FrancisLfg
Copy link
Contributor

@FrancisLfg FrancisLfg commented Aug 9, 2020

Some AWS service integration are not supported by the sdk, for instance ECS/Fargate with callback pattern : https://docs.aws.amazon.com/step-functions/latest/dg/connect-supported-services.html.

This PR improve service integration and support callback pattern for ECS/Fargate.

2 options has been removed : 'wait_for_callback' and 'wait_for_completion' and a new one has been added : 'integration_pattern'.
As we have 3 choices Request, Sync and Callback boolean option does not suit well this is why this PR adds an enum to manage the service integration pattern. So we now need to pass the wanted pattern as :

step = BatchSubmitJobStep('Batch Job', integration_pattern=IntegrationPattern.RequestResponse)
where integration_pattern can take following values :

  • IntegrationPattern.RequestResponse
  • IntegrationPattern.RunAJob
  • IntegrationPattern.WaitForCallback

I have used documentation terminology. It is a breaking change but IMHO, the 2 options wait_for_* should not be kept.

I also refactored a bit Task class to check if pattern if supported by a given service, and to automatize Field.Resource.value generation.

All unit tests are OK, I am not able to perform integ test right now but it should have no impact.
I am not sure how to test the documentation generation, what should I do to check that documentation is OK ?

Let me know if some part are not clear or if we could do better, I am not sure about the "name" and "action" Task class attribute but it helps simplifying Field.Resource.value value generation and error management, what do you think ?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: StepFunctionsPythonSDK-integtests
  • Commit ID: a03b9c3
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@StepFunctions-Bot
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: StepFunctionsPythonSDK-integtests
  • Commit ID: b342696
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Base automatically changed from master to main February 25, 2021 21:10
@creatorrr
Copy link

Is this planned to be merged soon? PR seems a bit outdated, @FrancisLfg do you need help to update it?

@shivlaks
Copy link
Contributor

shivlaks commented Oct 5, 2021

Is this planned to be merged soon? PR seems a bit outdated, @FrancisLfg do you need help to update it?

@creatorrr - this PR does seem a little stale, but we are starting to introduce integration patterns as an enum in #125
We will need to resolve the open thread around how we introduce the patterns to existing service integrations.

We cannot just change the properties and drop the flags to introduce the integration pattern enum as it breaking.
We will need to introduce it as an optional property and mark the flags for deprecation (which we can subsequently flush out when we release v3)

copy @ca-nguyen

@FrancisLfg FrancisLfg closed this Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants