-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[ACS] format default dns_name_prefix #4237
Conversation
@rjtsdl, |
fix #4116 |
cff495b
to
3648377
Compare
@anhowe TAL |
Codecov Report
@@ Coverage Diff @@
## master #4237 +/- ##
==========================================
+ Coverage 70.05% 70.05% +<.01%
==========================================
Files 475 475
Lines 29605 29612 +7
Branches 4539 4540 +1
==========================================
+ Hits 20739 20746 +7
Misses 7419 7419
Partials 1447 1447
Continue to review full report at Codecov.
|
@@ -81,7 +81,7 @@ def _get_feature_in_preview_message(): | |||
# some admin names are prohibited in acs, such as root, admin, etc. Because we have no control on the orchestrators, so default to a safe name. | |||
register_cli_argument('acs', 'admin_username', options_list=('--admin-username',), default='azureuser', required=False) | |||
register_cli_argument('acs', 'api_version', options_list=('--api-version',), required=False, help=_get_feature_in_preview_message() + 'Use API version of ACS to perform az acs operations. Available options: 2017-01-31, 2017-07-01. Default to use the latest version for the location') | |||
register_cli_argument('acs', 'dns_name_prefix', options_list=('--dns-prefix', '-d')) | |||
register_cli_argument('acs', 'dns_name_prefix', options_list=('--dns-prefix', '-d'), help='default use the format of <clustername>-<resourcegroupname>-<subid>, will trim the length and replace sensitive charactors if needed') |
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.
typo: characters
if not name_part[0].isalpha(): | ||
name_part = (str('a') + name_part)[0:10] | ||
resource_group_part = re.sub('[^A-Za-z0-9-]', '', resource_group_name)[0:16] | ||
dns_name_prefix = '{}-{}-{}'.format(name_part, resource_group_part, subscription_id[0:6]) |
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.
I'd recommend extracting this logic to method and adding a few unit tests.
4457a4d
to
c57559e
Compare
@tjprescott , please help merge if you approve :) |
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.
LGTM. Thanks!
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Command Guidelines
(see Authoring Command Modules)