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

feat: Add tags to dynamodb config #4100

Merged

Conversation

nanohanno
Copy link
Contributor

What this PR does / why we need it:

Add the option to configure and set tags on the AWS DynamoDB table resource created for an online store. Many organizations use tags to manage their resources which makes it convenient to directly add tags in the configuration file of Feast so that they are added upon creation of the tables.

Which issue(s) this PR fixes:

Fixes

Copy link
Member

@franciscojavierarceo franciscojavierarceo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

auto-merge was automatically disabled April 16, 2024 14:56

Head branch was pushed to by a user without write access

@nanohanno
Copy link
Contributor Author

The integration test failed due to an aws iam permission error. It seems that even when the Tags attribute in the CreateTable call is an empty array a call to the TagResource API is made which needs more permissions.

I think this is not only an issue of the integration test but in general it would break existing setups. The permission to add tags should only be required if tags are actually configured in the config file for DynamoDB.

I made a change that should avoid such an issue and add the Tags attribute only when configured in the file.

@franciscojavierarceo franciscojavierarceo enabled auto-merge (squash) April 17, 2024 12:36
@franciscojavierarceo franciscojavierarceo merged commit b08b8d5 into feast-dev:master Apr 17, 2024
15 checks passed
jeremyary pushed a commit that referenced this pull request Apr 17, 2024
# [0.37.0](v0.36.0...v0.37.0) (2024-04-17)

### Bug Fixes

* Pgvector patch ([#4103](#4103)) ([5c4a9c5](5c4a9c5))
* Remove top-level grpc import in cli ([#4107](#4107)) ([4362b6c](4362b6c))

### Features

* Add tags to dynamodb config ([#4100](#4100)) ([b08b8d5](b08b8d5))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants