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

[Communication] - Phone Numbers - Changed User-Agent in request #17162

Merged
merged 1 commit into from
Mar 8, 2021

Conversation

jbeauregardb
Copy link
Contributor

No description provided.

@jbeauregardb
Copy link
Contributor Author

/azp run python - communication - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jbeauregardb
Copy link
Contributor Author

/azp run python - communication - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jbeauregardb
Copy link
Contributor Author

/azp run python - communication - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

item['phoneNumber'] = self._replacement
elif "id" in item:
Copy link

Choose a reason for hiding this comment

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

why we change from elif to if here?

Copy link

@sacheu sacheu left a comment

Choose a reason for hiding this comment

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

i left a few comments.

@@ -25,14 +25,14 @@ def process_response(self, response):
if isinstance(item, str):
body["phoneNumbers"] = [self._replacement]
break
elif "phoneNumber" in item:
Copy link

Choose a reason for hiding this comment

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

why we change from elif to if here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to sanitize both values. Having an elif only sanitized the phone number or the id, but not both at the same time.

item['id'] = self._replacement
response['body']['string'] = json.dumps(body)
response['url'] = self._replacement
return response
except (KeyError, ValueError):
except (KeyError, ValueError, TypeError):
Copy link

Choose a reason for hiding this comment

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

why we need this change for the User-agent change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a fix for the type error issue we have in the search available phone numbers test. We weren't catching that exception before. It's totally unrelated to the user agent issue, though.

Copy link

@sacheu sacheu left a comment

Choose a reason for hiding this comment

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

Thanks. Looks good.

@jbeauregardb jbeauregardb merged commit 92c46c2 into Azure:master Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants