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

It is not possible to remove TableNamePrefix via DynamoDBOperationConfig #3470

Closed
Dreamescaper opened this issue Sep 9, 2024 · 7 comments
Closed
Labels
bug This issue is a bug. dynamodb p2 This is a standard priority issue queued v4

Comments

@Dreamescaper
Copy link
Contributor

Dreamescaper commented Sep 9, 2024

Describe the bug

	        [DynamoDBTable("test-table")]
	        class ExternalTable
	        {
				...
	        }

            var dynamoDbContext = new DynamoDBContext(new DynamoDBContextConfig
            {
                TableNamePrefix = "my-prefix-"
            });

            dynamoDbContext.SaveAsync(new ExternalTable { }, new DynamoDBOperationConfig
            {
                TableNamePrefix = "",
                OverrideTableName = "external-table"
            });

Expected Behavior

Since I have explicitly set TableNamePrefix to empty string, I would expect it to be empty, there SaveAsync would happen for "external-table" table name.

Current Behavior

TableNamePrefix = "" is ignored. SaveAsync is attempted for "my-prefix-external-table" table name.

Possible Solution

Create a separate context.

Additional Information/Context

DynamoDBFlatConfig uses string.IsNullOrEmpty comparison, therefore empty string is simply ignored.

AWS .NET SDK and/or Package version used

3.7.402.7

Targeted .NET Platform

.NET 8

Operating System and version

AmazonLinux

@Dreamescaper Dreamescaper added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 9, 2024
@Dreamescaper Dreamescaper changed the title It is not possible to remove TableNamePrefix via It is not possible to remove TableNamePrefix via DynamoDBOperationConfig Sep 9, 2024
@bhoradc bhoradc added needs-reproduction This issue needs reproduction. dynamodb p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Sep 9, 2024
@bhoradc bhoradc self-assigned this Sep 9, 2024
@bhoradc
Copy link

bhoradc commented Sep 10, 2024

Hello @Dreamescaper,

Thank you for reporting the issue. As per the source code, if the DynamoDBOperationConfig.TableNamePrefix is Empty it falls back to the DynamoDBContextConfig.TableNamePrefix which is set to "my-prefix-" in your case.

Ideally DynamoDBOperationConfig would override any settings specified by the DynamoDBContextConfig object. But due to above logic, I don’t see a way to remove the TableNamePrefix from inside DynamoDBOperationConfig if its set (Non Empty or not null) in DynamoDBContextConfig.

I will further check on this with the team and keep you posted.

Repro code

Package version: AWSSDK.DynamoDBv2 3.7.400.18

       static async Task Main(string[] args)
       {
           //Create DDB table=`my-prefix-external-table` and Partition key as id(String)

           var client = new AmazonDynamoDBClient();

           var config = new DynamoDBContextConfig
           {
               TableNamePrefix = "my-prefix-"
           };

           var dynamoDbContext = new DynamoDBContext(client, config);

           await dynamoDbContext.SaveAsync(new ExternalTable { Id = "1" }, new DynamoDBOperationConfig
           {
               TableNamePrefix = "",
               OverrideTableName = "external-table"
           });
       }

       [DynamoDBTable("test-table")]
       public class ExternalTable
       {
           [DynamoDBHashKey]
           [DynamoDBProperty("id")]
           public string Id { get; set; } = null!;
       }

Regards,
Chaitanya

@bhoradc bhoradc added needs-review and removed needs-reproduction This issue needs reproduction. labels Sep 10, 2024
@Dreamescaper
Copy link
Contributor Author

I assume that technically, changing that behavior would be a breaking change.
Would it make sense to fix it in V4 for new operation-specific overloads?

@bhoradc bhoradc added v4 and removed needs-review labels Sep 13, 2024
@bhoradc
Copy link

bhoradc commented Sep 16, 2024

Hello @Dreamescaper,

Team will be fixing this issue as part of V4 efforts. Thank you again for reporting the issue.

Regards,
Chaitanya

@bhoradc bhoradc added the queued label Sep 16, 2024
@bhoradc bhoradc removed their assignment Sep 16, 2024
@Dreamescaper
Copy link
Contributor Author

Dreamescaper commented Sep 17, 2024

@bhoradc
Would it be fine for me to implement this?

@normj
Copy link
Member

normj commented Sep 17, 2024

@Dreamescaper happy to take a PR for this change. Just be sure to target the v4-development branch.

@dscpinheiro
Copy link
Contributor

Thanks again for the contribution, your PR was included in the latest preview of the SDK that was just released: https://www.nuget.org/packages/AWSSDK.DynamoDBv2/4.0.0-preview.3

Copy link

github-actions bot commented Oct 1, 2024

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
bug This issue is a bug. dynamodb p2 This is a standard priority issue queued v4
Projects
None yet
Development

No branches or pull requests

4 participants