Skip to content
This repository has been archived by the owner. It is now read-only.

Extract constant for default sender endpoint #144

Merged
merged 1 commit into from Feb 1, 2019
Merged

Extract constant for default sender endpoint #144

merged 1 commit into from Feb 1, 2019

Conversation

@c-w
Copy link
Contributor

@c-w c-w commented Jan 30, 2019

This change brings three main benefits:

  1. Clients can now programmatically access the value of the default sender endpoint, for example to forward requests to it. This benefit is demonstrated by the changes to the test code.

  2. Avoid copy/paste of the endpoint URL between async and sync senders.

  3. Defaulting the service_endpoint_url to None instead of the string value and checking against null later to look up the default endpoint url means that the class now supports use-cases like sender = AsynchronousSender(os.getenv('APPINSIGHTS_ENDPOINT_URL')) which simplifies client code that may want to optionally override the service url. This benefit is demonstrated by the changes to the Django and Flask integrations.

This change brings three main benefits:

1) Clients can now programmatically access the value of the default
   sender endpoint, for example to forward requests to it. This benefit
   is demonstrated by the changes to the test code.

2) Avoid copy/paste of the endpoint URL between async and sync senders.

3) Defaulting the service_endpoint_url to None instead of the string
   value and checking against null later to look up the default endpoint
   url means that the class now supports use-cases like
   `Sender(os.getenv('SERVICE_URL'))` which simplifies client code that
   may want to optionally override the service url. This benefit is
   demonstrated by the changes to the Django and Flask integrations.
@SergeyKanzhelev
Copy link
Contributor

@SergeyKanzhelev SergeyKanzhelev commented Feb 1, 2019

sorry for delay. I'll also add a line to CHANGELOG.md.

@SergeyKanzhelev SergeyKanzhelev merged commit 520d11c into microsoft:develop Feb 1, 2019
2 checks passed
@c-w c-w deleted the default-endpoint-url branch Feb 1, 2019
@c-w
Copy link
Contributor Author

@c-w c-w commented Feb 1, 2019

Thanks for the merge!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants