Skip to content

Introduce CustomHosts option #45

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.

Sign up for GitHub

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

Merged
nikithauc merged 5 commits into from
May 20, 2021
Merged

Introduce CustomHosts option #45

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
28cebe8
Adding custom urls requirement.
nikithauc May 3, 2021
67600ec
Update AuthorizationHandler.md
nikithauc May 3, 2021
dda579b
Update TelemetryHandler.md
nikithauc May 3, 2021
888fccb
Update GraphClientFactory.md
nikithauc May 12, 2021
df6750a
updating to customhosts
nikithauc May 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion GraphClientFactory.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,14 @@ There are two primary usage scenarios for the client factory. Developers who wi
- Set the SdkVersion header with the appropriate moniker based on the following structure `graph-{lang}-{version}`.
- Enable the developer to select a supported sovereign cloud using an enumerated list. Selecting the sovereign cloud should ensure that the AuthenticationProvider uses the appropriate Authentication Endpoint.
- Enable a developer to configure a HTTP proxy that will be used for outgoing requests.

- Enable a developer provide custom URLs or endpoints.

@MIchaelMainer MIchaelMainer May 3, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Enable a developer provide custom URLs or endpoints.
Enable a developer to provide custom URLs or endpoints.

I think we should consider using the RequestContext mechanism to pass through the CustomUrls option through the middleware, in addition to an on individual request option where we can disable applying our headers.

Should this only be applicable to our default middleware or should the mechanism be accessible to custom middleware? Ideally custom middleware should be able to take advantage of this.

Do you have an example of a workload failing with an unexpected header? I can only think of OneDrive on redirection to a download/upload URL.

@nikithauc nikithauc May 4, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

An example of a workload failing with an unexpected header - I got this error with Outlook large file upload too.

Comment thread
nikithauc marked this conversation as resolved.
Outdated
- `CustomURLs` option should be set on client creation.
- These URLs are different from the Graph service endpoints on the national clouds.

@baywet baywet May 17, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- These URLs are different from the Graph service endpoints on the national clouds.
- These hostnames are different from the Graph service endpoints on the national clouds.

- Certain workloads error out when an unexpected header is present in the request. The middlewares should check:
- If the request URL is a Graph serve endpoint or a custom url provided by the developer, then append request headers or modify the request content.
Comment thread
nikithauc marked this conversation as resolved.
Outdated
- Else the middleware should delete request headers added by that middleware.
Comment thread
MIchaelMainer marked this conversation as resolved.
- Provide capabilities to modify or update the `CustomURLs` option after the client creation.

@baywet baywet May 17, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- Provide capabilities to modify or update the `CustomURLs` option after the client creation.
- Provide capabilities to modify or update the `CustomHosts` option after the client creation.


## Performance Considerations

- If available on the platform, enable gzip compression of requests and responses.
Expand Down
4 changes: 4 additions & 0 deletions middleware/AuthorizationHandler.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ A piece of client-side middleware intended to allow the use of an authorization
- Transparently authorize requests
- Accept AuthorizationProvider instance in constructor
- Update Authorization Header
- Certain workloads error out when an unexpected header is present in the request. The middlewares should check:
- If the request URL is a Graph serve endpoint or a custom url provided by the developer, then append or update the authorization header.
- Else the middleware should delete authorization header.


Take a request object and use authorization provider to add Authorization header.

Expand Down
4 changes: 4 additions & 0 deletions middleware/TelemetryHandler.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ Provide a mandatory middleware component that attaches metadata to a Graph reque
- `RuntimeEnvironment: Node/1.1` for JavaScript.
- `RuntimeEnvironment: JRE/1.1` for Java.

- Certain workloads error out when an unexpected header is present in the request. The middlewares should check:
Comment thread
andrueastman marked this conversation as resolved.
- If the request URL is a Graph serve endpoint or a custom url provided by the developer, then append or update the telemetry headers.
- Else the middleware should delete telemetry header.

#### Ideal Metadata Capture
```
SdkVersion: graph-dotnet-beta/0.6.0-preview, graph-dotnet-core/1.16.0 (featureUsage=0f)
Expand Down