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

add sku option for sqldatabase #291

Merged
merged 13 commits into from
Nov 6, 2020
Merged

add sku option for sqldatabase #291

merged 13 commits into from
Nov 6, 2020

Conversation

guyomog78
Copy link
Contributor

@guyomog78 guyomog78 commented Oct 13, 2020

SUMMARY

Change the non functional edition option to a functional sku option.

Partial Fixes #153 Unable to specify a SKU
Fixes #126

ISSUE TYPE
  • Feature Pull Request
  • Bugfix Pull Request
COMPONENT NAME

azure_rm_sqldatabase.py
azure_rm_sqldatabase_info.py

ADDITIONAL INFORMATION

The Sku option use the Python Sku class from azure.mgmt.sql.models to be able to push valide values.
The documentation is updated with the new SKU call option
The internal test to detect change is updated (replace edition by sku)

The sku entrie is then identical as the azure_rm_sqldatabase_info report

example call before change

  - name: Create (or update) SQL Database with SKU
    azure_rm_sqldatabase:
      resource_group: myResourceGroup
      server_name: sqlcrudtest-5961
      name: testdb
      location: eastus
      edition: standard

example call after change

  - name: Create (or update) SQL Database with SKU
    azure_rm_sqldatabase:
      resource_group: myResourceGroup
      server_name: sqlcrudtest-5961
      name: testdb
      location: eastus
      sku:
        name: S0

or with more options
example call after change

  - name: Create (or update) SQL Database with SKU
    azure_rm_sqldatabase:
      resource_group: myResourceGroup
      server_name: sqlcrudtest-5961
      name: testdb
      location: eastus
      sku:
        name: S0
        tier: Standard
        capacity: 50

Attention All option under sku should be coherent with the name value else an error is reported

guyomog78 and others added 4 commits September 30, 2020 18:21
start SKU minimum implementation
Functionnal with basic and standard need to improv for S0, S1 ...
Replace the old non functionnal option **Edition**
by the **SKU** Dict
@Fred-sun Fred-sun added medium_priority Medium priority work in In trying to solve, or in working with contributors labels Oct 15, 2020
plugins/modules/azure_rm_sqldatabase.py Show resolved Hide resolved
plugins/modules/azure_rm_sqldatabase.py Outdated Show resolved Hide resolved
plugins/modules/azure_rm_sqldatabase.py Outdated Show resolved Hide resolved
plugins/modules/azure_rm_sqldatabase.py Outdated Show resolved Hide resolved
@Fred-sun
Copy link
Collaborator

@guyomog78 Can you help tests/integration/the targets/azure_rm_sqlserver/tasks/main yml increase test cases? Thank you very much!

@guyomog78
Copy link
Contributor Author

@Fred-sun Like make a test with the S0 mode and ensure that realy is a S0 mode, then change to P1 mode ensure that is well a P1?? someting like that?

@Fred-sun
Copy link
Collaborator

@Fred-sun Like make a test with the S0 mode and ensure that realy is a S0 mode, then change to P1 mode ensure that is well a P1?? someting like that?

yes! Thanks!

@guyomog78
Copy link
Contributor Author

OK, did you have an ETA in mind?

Ensure that the sku name is name as requested
 - use current_service_objective_name instead of sku.name
 - sku.name is too generic
Add validation test for SKU usage
@guyomog78
Copy link
Contributor Author

@Fred-sun, it was a little bit more difficulte as expected, but necessary to correct the sqldatabasinfo too to report the good SKU like documentation say.
when database information collected, sku name is equal to Standard for Service_objective_name S0,S1,S2.. Premium for Service_objective_name P1, P2 ,P3 ...
So, to use the expected information in sku.name (ex: S0, P2, DW100c , GP_FsV2_12...) I have used the current_service_objective_name retruned by the request.

To see current information: Get-AzSqlServerServiceObjective -Location eastus

@Fred-sun
Copy link
Collaborator

Fred-sun commented Nov 5, 2020

@guyomog78 Such parameter changes may cause the edition parameter previously configured by the user to be unavailable. Can the following two methods be used to avoid this problem?

First: Add Fail prompt. After this version, the edition parameter is changed to sku, and sku aliase is changed to sku.

Second: Keep the edition parameter, but set the default value and "mutually_exclusive=['edition', 'sku']", for example, when edition: Standard, the default sku.name=S1, and edition: premium, the default sku.name=P1.

@guyomog78
Copy link
Contributor Author

@Fred-sun t it is a while that the edition parameter is not functionnal, but I understand your point of view.

I will try to keep for this time the edition parameters

 - edition still present, but finally use arbitrary SKU
  - choice edition or Sku
  - update test to test edition too on first test
    - remember to remove next version
@guyomog78
Copy link
Contributor Author

guyomog78 commented Nov 5, 2020

@Fred-sun It's Done!!
I have also used autopep8 and pylint in vscode and applied "format document"

On test file, I have injected Edition value Premium and it's good. I have tested with standard too ;-)

et Voilà!

PS: Feel free to update the version where edition will be full removed

plugins/modules/azure_rm_sqldatabase.py Outdated Show resolved Hide resolved
plugins/modules/azure_rm_sqldatabase.py Outdated Show resolved Hide resolved
plugins/modules/azure_rm_sqldatabase.py Outdated Show resolved Hide resolved
@guyomog78
Copy link
Contributor Author

@Fred-sun Thanks a lot for review.
Now I'm aware about that the import part for script should be after documentation. It was modified by the "format document" under vscode. Now, I know that I need to revert this part for the next time if appens :-)

@Fred-sun Fred-sun added ready_for_review The PR has been modified and can be reviewed and merged and removed work in In trying to solve, or in working with contributors labels Nov 6, 2020
@haiyuazhang haiyuazhang merged commit 69fe33a into ansible-collections:dev Nov 6, 2020
@guyomog78 guyomog78 deleted the fix-SQLSKU branch November 6, 2020 08:47
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.

azure_rm_sqldatabase lacks critical functionality azure_rm_sqldatabase - edition don't work
3 participants