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

dynamodb_table: billing_mode, sse, gsi fixes #1

Closed
wants to merge 1 commit into from
Closed

dynamodb_table: billing_mode, sse, gsi fixes #1

wants to merge 1 commit into from

Conversation

marknet15
Copy link

SUMMARY

Providing the fixes listed below for the changes proposed in ansible#68320 that are now stale that is in turn based on the below:

Fix AnsibleAwsModule migration stale PR (ansible#45402). Add billing_mode and point_in_time_recovery. > Small bug fixes. Integration tests fixes.

Fixes:

  • Fix issue with ProvisionedThroughput params being set incorrectly on GSI's when billing is PAY_PER_REQUEST.
  • Fix incorrect key_type_mapping.
  • Fix defining of serialized_index_attribute_definitions for both the hash and the range key on a GSI. (range key definition was being overwritten).
  • Fix defining of SSE spec.
  • Remove need for dict_merge function that provided some errors.

Fixes ansible#45402
Fixes ansible#56923

ISSUE TYPE
  • Bugfix Pull Request
  • Feature Pull Request
COMPONENT NAME
  • dynamodb_table
ADDITIONAL INFORMATION
- name: enable on-demand billing and backups
  dynamodb_table:
  name: table_name
  hash_key_name: myhashkey
  range_key_name: myrangekey
  billing_mode: PAY_PER_REQUEST
  point_in_time_recovery: yes
  indexes:
      - name: myindex
          type: global_all
          hash_key_name: somekey
          range_key_name: myrangekey

* Fix issue with `ProvisionedThroughput` params being set incorrectly on GSI's when billing is `ON_DEMAND`
* Fix incorrect `key_type_mapping`
* Fix defining of `serialized_index_attribute_definitions` for both the hash and the range key on a GSI. (range key definition was being overwritten).
* Fix defining of SSE spec.
* Remove need for `dict_merge` function that provided some errors.
@anryko
Copy link
Owner

anryko commented May 2, 2020

I'll take a closer look in a few days, beginning of the next week. From fast glance it looks good, the only thing that caught my eye right away is dict merges and comprehensions that break python 2 compatibility.

@marknet15
Copy link
Author

@anryko thanks :) So on the main ansible PR one of the engineers has replied on the PR and has said that the aws modules have been moved out of the repo and that this PR would have to be re-created there. If you like I can get this started ?

On your comment around the dict merges I can revert that bit for now and leave it out as I hadn't thought about py2.

@anryko
Copy link
Owner

anryko commented May 4, 2020

@marknet15 good! If you would like to take it over and resubmit all of the commits as a new PR, run with it. If you'll need any assistance with it, feel free to ping me in the "Conversation" of that new PR and I'll be happy to help.

@marknet15
Copy link
Author

marknet15 commented May 4, 2020

Awesome ok thanks @anryko I will get the new PR created :) yep will do !

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.

2 participants