[Network] New Traffic Manager features #8188
Conversation
marstr
left a comment
marstr
left a comment
There was a problem hiding this comment.
Few questions, but nothing that certainly requires changes.
| for item in namespace.monitor_custom_headers: | ||
| try: | ||
| item_split = item.split('=', 1) | ||
| values.append({'name': item_split[0], 'value': item_split[1]}) |
There was a problem hiding this comment.
If values are allowed to have escaped equals signs, then we'll need a more robust split strategy here. If it's always a base64 encoded or similar, then we're okay.
There was a problem hiding this comment.
This is the same strategy used that has always been used with --tags and we've not had complaints. I believe it is because of the simpler constraints on the key. If there's an equal sign in the value (escaped or not) it shouldn't be an issue.
There was a problem hiding this comment.
Err, so I may not need to worry about this. If the headers have already been correctly URL encoded, any equal sign should just show up as %3D. I imagine we're okay requiring properly encoded info here.
There was a problem hiding this comment.
(sorry, looks like there was a race condition, and I didn't see your message when I was typing mine.)
| values = [] | ||
| for item in namespace.status_code_ranges: | ||
| item_split = item.split('-', 1) | ||
| usage_error = CLIError('usage error: --status-code-ranges VAL | --status-code-ranges MIN-MAX') |
There was a problem hiding this comment.
Are MIN and MAX inclusive or exclusive? As written, I assume inclusive.
There was a problem hiding this comment.
The SDK is ambiguous on this, but yes I would assume inclusive. If you give a single value, it simply uses that value as both min and max.
| values = [] | ||
| for item in namespace.subnets: | ||
| try: | ||
| item_split = item.split('-', 1) |
There was a problem hiding this comment.
Unlike above, I think subnet strings are constrained enough that I don't think it's a problem to just use a split.
|
@marstr replied to you comments. If you are happy with the responses, feel free to merge and proceed with the release. :) |
3 participants
Closes #7940.
This checklist is used to make sure that common guidelines for a pull request are followed.
The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).
I adhere to the Command Guidelines.