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

(stepfunctions-tasks): support parameters in StepFunctionsInvokeActivity #21020

Closed
eiva opened this issue Jul 6, 2022 · 4 comments · Fixed by #21077
Closed

(stepfunctions-tasks): support parameters in StepFunctionsInvokeActivity #21020

eiva opened this issue Jul 6, 2022 · 4 comments · Fixed by #21077
Labels
@aws-cdk/aws-stepfunctions-tasks effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@eiva
Copy link

eiva commented Jul 6, 2022

Describe the bug

You already closed same ticket in CDK1 and probably because of this ignorance to customer pain never added it to v2.

Here is a ticket: #1480

It is not even feature request - it is already a bug in cdk 2 as with dropping arbitrary Task class you actually made impossible to have this parameter specified for Activities.

I know that this ticket will be auto closed as many others as usual.

But this is a nonsense.

Or at least spent some of your time to provide workaround if supporting something what here for years is too hard for the CDK team.

Expected Behavior

software.amazon.awscdk.services.stepfunctions.tasks.StepFunctionsInvokeActivity should have parameters.

Current Behavior

software.amazon.awscdk.services.stepfunctions.tasks.StepFunctionsInvokeActivity does not have parameters

Task is deprecated. You make things impossible for migration.

Reproduction Steps

It is not even required to run anything.

Just go and try to use parameters for software.amazon.awscdk.services.stepfunctions.tasks.StepFunctionsInvokeActivity

Possible Solution

Will really appreciate if you provide any.

Additional Information/Context

No response

CDK CLI Version

2

Framework Version

No response

Node.js Version

16

OS

linux

Language

Typescript

Language Version

No response

Other information

No response

@eiva eiva added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 6, 2022
@github-actions github-actions bot added the @aws-cdk/aws-stepfunctions Related to AWS StepFunctions label Jul 6, 2022
@eiva eiva changed the title StepFunctions: Missing support of SF parameters in software.amazon.awscdk.services.stepfunctions.tasks.StepFunctionsInvokeActivity StepFunctions: Regression: Missing support of SF parameters in software.amazon.awscdk.services.stepfunctions.tasks.StepFunctionsInvokeActivity Jul 6, 2022
@kaizencc kaizencc changed the title StepFunctions: Regression: Missing support of SF parameters in software.amazon.awscdk.services.stepfunctions.tasks.StepFunctionsInvokeActivity (stepfunctions-tasks): support parameters in StepFunctionsInvokeActivity Jul 7, 2022
@kaizencc kaizencc added feature-request A feature should be added or improved. @aws-cdk/aws-stepfunctions-tasks and removed bug This issue is a bug. @aws-cdk/aws-stepfunctions Related to AWS StepFunctions labels Jul 7, 2022
@kaizencc
Copy link
Contributor

kaizencc commented Jul 7, 2022

There is always a CustomState where you can drop down into raw json and provide whatever you want:

https://github.com/aws/aws-cdk/blob/main/packages/@aws-cdk/aws-stepfunctions/README.md#custom-state

There is no regression here. Repurposing this issue as a feature request.

@kaizencc kaizencc added good first issue Related to contributions. See CONTRIBUTING.md p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jul 7, 2022
@kaizencc kaizencc removed their assignment Jul 7, 2022
@dlegezo
Copy link

dlegezo commented Jul 7, 2022

There is a dramatic difference between State and Task in V1:
When you used task there was an ability to create instance and than you still able to get metrics, addCatch, addNext and so on.
With State class you suggest - this all is impossible (you have to provide catches and next in chain right when you generate it) so the only way that you need to assemble all state machines in completely different way. When you have dozens of SFs with many-many activities it become impossible as will require to re-implement this all from scratch. Literally if this is not a regression in system - I dont know how to call it.

And it is not a feature. You should being use base interface for all things derived from task to not miss such things and still leave ability to interact with generated json for tasks as well rather creating huge amount of specific tasks with no proper class hierarchy.

@kaizencc
Copy link
Contributor

kaizencc commented Jul 7, 2022

You can always extend TaskStateBase yourself if you want to use the methods associated with it:

import { Stack, StackProps } from 'aws-cdk-lib';
import { Construct } from 'constructs';
import * as iam from 'aws-cdk-lib/aws-iam';
import * as sfn from 'aws-cdk-lib/aws-stepfunctions';

export class CustomStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const task = new CustomTask(this, 'id', {});
    task.addCatch(new sfn.Fail(this, 'fail'));
    const machine = new sfn.StateMachine(this, 'machine', {
      definition: task.next(new sfn.Succeed(this, 'success')),
    });
  }
}

interface CustomProps extends sfn.TaskStateBaseProps {}

class CustomTask extends sfn.TaskStateBase {
  protected readonly taskMetrics?: sfn.TaskMetricsConfig;
  protected readonly taskPolicies?: iam.PolicyStatement[];
  
  constructor(scope: Construct, id: string, private readonly props: CustomProps) {
    super(scope, id, props);
  }
  
  protected _renderTask(): any {
    return {};
  }
}

@mergify mergify bot closed this as completed in #21077 Jul 12, 2022
mergify bot pushed a commit that referenced this issue Jul 12, 2022
…ctivity (#21077)

Fixes #21020

This PR adds the parameters to the StepFunctionsInvokeActivity class.

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-stepfunctions-tasks effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants