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

azure_rm_sqldatabase: parse datetime module arguments #623

Merged
merged 8 commits into from
Feb 25, 2022

Conversation

nbr23
Copy link
Contributor

@nbr23 nbr23 commented Aug 29, 2021

SUMMARY

In azure_rm_sqldatabase, the arguments restore_point_in_time and source_database_deletion_date are dates.
This bugfix parses the string passed as argument, and creates the datetime object required.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • azure_rm_sqldatabase
ADDITIONAL INFORMATION

Currently the module expects datetime objtects to be passed directly as module arguments from the playbook. However, it appears that argument_spec does not handle datetime as an input type: https://docs.ansible.com/ansible/latest/dev_guide/developing_program_flow_modules.html#argument-spec

To correct this, this fix changed the type of restore_point_in_time and source_database_deletion_date to str, and calls dateutil.parse on the string to get the datetime object expected by the azure sdk.

This fixes #619

@Fred-sun
Copy link
Collaborator

tests/sanity/ignore-2.10.txt:273:1: A100: Ignoring 'invalid-ansiblemodule-schema' on 'plugins/modules/azure_rm_sqldatabase.py' is unnecessary

@nbr23
Copy link
Contributor Author

nbr23 commented Aug 30, 2021

tests/sanity/ignore-2.10.txt:273:1: A100: Ignoring 'invalid-ansiblemodule-schema' on 'plugins/modules/azure_rm_sqldatabase.py' is unnecessary

Removed, thanks!

@Fred-sun
Copy link
Collaborator

tests/sanity/ignore-2.10.txt:273:1: A100: Ignoring 'invalid-ansiblemodule-schema' on 'plugins/modules/azure_rm_sqldatabase.py' is unnecessary

Same operation (tests/sanity/ignore-2.11.txt and tests/sanity/ignore-2.12.txt)

@Fred-sun

This comment has been minimized.

plugins/modules/azure_rm_sqldatabase.py Show resolved Hide resolved
@@ -224,6 +224,7 @@

import time
from ansible_collections.azure.azcollection.plugins.module_utils.azure_rm_common import AzureRMModuleBase, format_resource_id
import dateutil.parser
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
import dateutil.parser

@Fred-sun
Copy link
Collaborator

@nbr23 Can you add test cases for newly added logic? Thank you very much!

@nbr23
Copy link
Contributor Author

nbr23 commented Sep 1, 2021

@Fred-sun test case added. I had to add a fact to the returned value of sqldatabase_info so I can make sure there is a restore point (and get its date): ed903a9

@Fred-sun Fred-sun added medium_priority Medium priority ready_for_review The PR has been modified and can be reviewed and merged labels Sep 1, 2021
@xuzhang3
Copy link
Collaborator

LGTM

@xuzhang3 xuzhang3 merged commit 094fff8 into ansible-collections:dev Feb 25, 2022
Fred-sun added a commit to Fred-sun/ansible_collections_azure that referenced this pull request Mar 8, 2022
* azure_rm_sqldatabase: parse datetime module arguments (ansible-collections#623)

* rm_sqldatabase: parse datetime arguments

* Remove unused sanity test exception on rm_sqldatabase module schema

* Remove unused sanity test exception on rm_sqldatabase module schema bis

* sqldatabase: import dateutil in try/except

* Add dateutil install to test suite

* sqldatabase_info: Add earliest_restore_date value to returned facts

* sqldatabase: add point in time restore test

* Conditionally call non MSI auth when interacting with keyvault (ansible-collections#770)

* Added the VM status detection mechanism (ansible-collections#772)

* Set the parameter to a random number

* Update storage account name

Update azure_rm_virtualmachine vars

add new change

add new change 02

add new change 03

add new change 05

add new change 06

add new change 08

add new change09

update new

Update new 02

Improve code logic

* fix a typo error. related to ansible-collections#757 (ansible-collections#769)

* fix a typo error. related to ansible-collections#757

* remove unused line

Co-authored-by: Daniele Marcocci <daniele.marcocci@outlook.com>

* Update test region (ansible-collections#776)

Co-authored-by: Max <max@23.tf>
Co-authored-by: Daniele Marcocci <danielinor0x@hotmail.com>
Co-authored-by: Daniele Marcocci <daniele.marcocci@outlook.com>
xuzhang3 pushed a commit that referenced this pull request Mar 8, 2022
* Ugrade azure-mgmt-compute SDK to track2

* fix small

* Modify version from v2021-07-01 to v2020-04-01, no disk encryptions operation

* Update small

* fix azure_rm_diskencryption test fail

* fix azure_rm_diskencryption test fail02

* fix sanity error

* fix azure_rm_diskcryptionset test fail

* fix azure_rm_virtualmachinescalesetinstance_info bug

* fix azure_rm_virtualmachinescalesetinstance_info bug 02

* fix azure_rm_virtualmachien*extension test fail

* Update azure_rm_virtualmachinescalesetinstance func paramter to vm_instance_i_ds

* fix azure_rm_virtualmachinescalesetinstance test fail

* fix sanity test fail

* change exception type

* fix azure_rm_hostgroup module

* Update the code that throws the exception

* Merge dev to local branch (#10)

* azure_rm_sqldatabase: parse datetime module arguments (#623)

* rm_sqldatabase: parse datetime arguments

* Remove unused sanity test exception on rm_sqldatabase module schema

* Remove unused sanity test exception on rm_sqldatabase module schema bis

* sqldatabase: import dateutil in try/except

* Add dateutil install to test suite

* sqldatabase_info: Add earliest_restore_date value to returned facts

* sqldatabase: add point in time restore test

* Conditionally call non MSI auth when interacting with keyvault (#770)

* Added the VM status detection mechanism (#772)

* Set the parameter to a random number

* Update storage account name

Update azure_rm_virtualmachine vars

add new change

add new change 02

add new change 03

add new change 05

add new change 06

add new change 08

add new change09

update new

Update new 02

Improve code logic

* fix a typo error. related to #757 (#769)

* fix a typo error. related to #757

* remove unused line

Co-authored-by: Daniele Marcocci <daniele.marcocci@outlook.com>

* Update test region (#776)

Co-authored-by: Max <max@23.tf>
Co-authored-by: Daniele Marcocci <danielinor0x@hotmail.com>
Co-authored-by: Daniele Marcocci <daniele.marcocci@outlook.com>

* Revert "Merge dev to local branch (#10)" (#11)

This reverts commit 1dce8f3.

Co-authored-by: Max <max@23.tf>
Co-authored-by: Daniele Marcocci <danielinor0x@hotmail.com>
Co-authored-by: Daniele Marcocci <daniele.marcocci@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium_priority Medium priority ready_for_review The PR has been modified and can be reviewed and merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't use restore_point_in_time in azure_rm_sqldatabase module
3 participants