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

sns.Topic.fromTopicArn: importinig topic forces contentBasedDeduplication = false #29532

Closed
Onolisk opened this issue Mar 18, 2024 · 3 comments · Fixed by #29542
Closed

sns.Topic.fromTopicArn: importinig topic forces contentBasedDeduplication = false #29532

Onolisk opened this issue Mar 18, 2024 · 3 comments · Fixed by #29542
Labels
@aws-cdk/aws-sns Related to Amazon Simple Notification Service bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@Onolisk
Copy link

Onolisk commented Mar 18, 2024

Describe the bug

When importing a topic by Arn, the constructor sets contentBasedDeduplication = false.

This means that when we call the resource method getContentBasedDeduplication(), it returns false always.

Expected Behavior

I would expect that when the resource is imported, the ARN is checked to determine whether or not contentBasedDeduplication is enabled on the Topic, then return a result that contains this.

Current Behavior

Currently .getContentBasedDeduplication() always returns false, regardless of the configuration of the actual Topic imported. This is only for imported Topics. Newly created topics return the actual value of the property.

Reproduction Steps

JAVA code:

    // Existing Topic with ContentBasedDeduplication enabled
    final ITopic assetTopic = Topic.fromTopicArn(this, "asset topic","arn:aws:sns:us-east-1:123456789012:myTopic.fifo");
    // returns false
    System.out.println(assetTopic.getContentBasedDeduplication());
    // New Topic created with ContentBasedDeduplication enabled
    Topic newTopic = Topic.Builder.create(this,"new topic")
            .contentBasedDeduplication(true)
            .displayName("my new topic")
            .fifo(true)
            .build();
    //Returns true, correctly
    System.out.println(newTopic.getContentBasedDeduplication());    

Possible Solution

No response

Additional Information/Context

See linked commit for when the property was set. Not sure if there is possibly an underlying reason why this value needs to be false for imports, if so please let me know.

CDK CLI Version

2.131.0

Framework Version

No response

Node.js Version

NA

OS

Win11

Language

Java

Language Version

openjdk version "21.0.2" 2024-01-16 LTS

Other information

No response

@Onolisk Onolisk added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 18, 2024
@github-actions github-actions bot added the @aws-cdk/aws-sns Related to Amazon Simple Notification Service label Mar 18, 2024
@AmylDV
Copy link

AmylDV commented Mar 19, 2024

I have also run into this issue. Resulting in an inability to correctly form a state machine in CDK that uses the SNSPublish Task to publish to a FIFO topic.

@pahud
Copy link
Contributor

pahud commented Mar 20, 2024

Thanks for the report. Looks like #29542 is underway. We'll review the PR when it's ready.

@pahud pahud added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Mar 20, 2024
@mergify mergify bot closed this as completed in #29542 Apr 6, 2024
mergify bot pushed a commit that referenced this issue Apr 6, 2024
#29542)

Closes #29532.

----

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

github-actions bot commented Apr 6, 2024

⚠️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-sns Related to Amazon Simple Notification Service bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants