-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Sql Import/Export CLI commands and test #2538
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
Conversation
|
@nathannfan, |
|
How does SharedAccessKey work? Do you put the SAS value in as the --storage-key? What happens if you put the SAS key on the end of the file uri instead? |
azure-cli.pyproj
Outdated
| <Folder Include="command_modules\azure-cli-component\azure-cli-configure\azure\" /> | ||
| <Folder Include="command_modules\azure-cli-component\azure-cli-configure\azure\cli\" /> | ||
| <Folder Include="command_modules\azure-cli-component\azure-cli-configure\azure\cli\command_modules\" /> | ||
| <Folder Include="command_modules\azure-cli-component\azure-cli-configure\azure\cli\command_modules\configure\" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did these changes happen? Maybe these should be reverted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, must have happened during a merge.
Codecov Report
@@ Coverage Diff @@
## master #2538 +/- ##
==========================================
+ Coverage 72.1% 72.13% +0.03%
==========================================
Files 362 362
Lines 19791 19813 +22
Branches 2920 2920
==========================================
+ Hits 14270 14292 +22
+ Misses 4604 4600 -4
- Partials 917 921 +4
Continue to review full report at Codecov.
|
|
|
|
Can you make a swagger change to deweirdify this message? You can remove |
| 'ServersOperations') | ||
|
|
||
| with ServiceGroup(__name__, get_sql_servers_operation, server_operations) as s: | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpicking: this is not required.
| ' --storage-key {} --storage-key-type StorageAccessKey' | ||
| ' --storage-uri {}{}/testbacpac.bacpac' | ||
| .format(server, db_name2, rg, admin_password, admin_login, key, | ||
| storage_endpoint, container)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There aren't any assertion on the last two command execution except the exit code. Shall we add more assertions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
|
||
| loc_long = location_long_name | ||
| rg = resource_group | ||
| sa = storage_account |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these assignment just for the sake of a shorter name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None blocking, but not recommended. Longer variable names improve the readability. Reassign and shorten the names defeat the purpose.
| admin_password = 'SecretPassword123' | ||
| db_name = 'cliautomationdb01' | ||
| db_name2 = 'cliautomationdb02' | ||
| container = 'bacpacs' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the self.create_random_name for all these names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is random preferred? Db names only need to be unique within a server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, it should be fine.
|
|
||
| firewall_rule_1 = 'allowAllIps' | ||
| start_ip_address_1 = '0.0.0.0' | ||
| end_ip_address_1 = '255.255.255.255' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it work if you just allow azure ips? i.e. start_ip_address = 0.0.0.0, end_up_address = 0.0.0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does
|
Looks great overall, just curious how SAS keys work. Could you add examples for each key type to help doc? Take a look at my help.py changes in #2536 to see how to format examples. Test for SAS key would be nice to have but not required. |
|
@troydai Thanks! |
* Azure/master: (478 commits) vm live test: allow more valid power states on vmss test verifications (Azure#2564) rbac:catch more graph error (Azure#2567) appservice: support to create plan when create a webapp (Azure#2550) Update storage tests (Azure#2556) Change PEP8 check filter from whitelist to blacklist (Azure#2557) Add scenario tests documentation (Azure#2555) [ACS] Adding support for configuring a default ACS cluster (Azure#2554) [ACS] Provide a short name alias for the orchestrator type flag (Azure#2553) Sql Import/Export CLI commands and test (Azure#2538) Fix format bug. (Azure#2549) [VM/VMSS] Improved disk caching support (Azure#2522) VM/VMSS: incorporate credentials validation logic used by portal (Azure#2537) Script that creates packaged releases package archive (Azure#2508) Adding alias for defaults flag (Azure#2540) Add wait commands and --no-wait support (Azure#2524) choice list outside of named arguments (Azure#2521) Fixed test failure in test_sql_db_mgmt. (Azure#2530) core: support login using service principal with a cert (Azure#2457) Add note about being in preview (Azure#2512) vm:fix distro check mechanism used by disk encryption (Azure#2511) ...
Uh oh!
There was an error while loading. Please reload this page.