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

Add support to Quickstart for testing devices behind an HTTP proxy. #446

Merged
merged 5 commits into from
Oct 17, 2018

Conversation

arsing
Copy link
Member

@arsing arsing commented Oct 16, 2018

This change adds two new CLI parameters --proxy and --upstream-protocol.
They correspond to agent.env.https_proxy and agent.env.UpstreamProtocol
in config.yaml.

--proxy defaults to the https_proxy environment variable on the
Quickstart process itself, if it's set.

Also fixed JSON errors in the smoke test deployment templates that caused jq
to choke.

This change adds two new CLI parameters `--proxy` and `--upstream-protocol`.
They correspond to `agent.env.https_proxy` and `agent.env.UpstreamProtocol`
in config.yaml.

`--proxy` defaults to the `https_proxy` environment variable on the
`Quickstart` process itself, if it's set.

Also fixed JSON errors in the smoke test deployment templates that caused `jq`
to choke.
@arsing
Copy link
Member Author

arsing commented Oct 16, 2018

This isn't enough. The EventHubClient inside Details also needs to be configured to use TransportType.AmqpWebSockets since the default is Amqp. Probably the same for the Devices.ServiceClient This is now done.

@arsing arsing requested review from damonbarry and myagley October 16, 2018 23:23
smoke/IotEdgeQuickstart/details/Details.cs Show resolved Hide resolved
@@ -26,6 +26,6 @@ public Task ProcessEventsAsync(IEnumerable<EventData> events)
return Task.CompletedTask;
}
public Task ProcessErrorAsync(Exception error) => throw error;
public int MaxBatchSize { get; } = 10;
public int MaxBatchSize { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Who uses this setter?

Copy link
Member Author

Choose a reason for hiding this comment

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

The interface requires it.

Copy link
Member Author

Choose a reason for hiding this comment

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

There doesn't seem to be any reason why they changed it to require a setter. The IPartitionReceiver impl in the SDK only gets its value.

@arsing arsing merged commit 1217b83 into Azure:master Oct 17, 2018
@arsing arsing deleted the quickstart-proxy branch October 17, 2018 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants