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 generated helix api client library #116

Merged
merged 6 commits into from
Apr 17, 2018

Conversation

alexperovich
Copy link
Member

@alexperovich alexperovich commented Apr 13, 2018

Needed for #118

This is the C# client library that allows access to the helix api. The telemetry sending tool will be using this library.

@alexperovich alexperovich self-assigned this Apr 13, 2018
@@ -0,0 +1,39 @@

Microsoft Visual Studio Solution File, Format Version 12.00

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@@ -0,0 +1,18 @@
<Project Sdk="RoslynTools.RepoToolset">

This comment was marked as spam.

This comment was marked as spam.

@chcosta
Copy link
Member

chcosta commented Apr 17, 2018

I thought this was for a general telemetry client usable by .NET products to report status, is this a Helix specific thing? All of the references to Helix are throwing me off as to what this is going to be used for.

@alexperovich
Copy link
Member Author

No, this is specifically for builds sending telemetry to helix so they can show up on MC.


<ItemGroup>
<PackageReference Include="Microsoft.Rest.ClientRuntime" Version="3.0.3" />
<PackageReference Include="Newtonsoft.Json" Version="10.0.3" />

This comment was marked as spam.

@chcosta
Copy link
Member

chcosta commented Apr 17, 2018

Maybe I don't have a clear picture of where Helix plays into the model. I thought the telemetry was going to MC and that this tool would be used by CI, official builds, etc... Where can I see the design so that I can understand why we need to call this a Helix thing?

@alexperovich
Copy link
Member Author

alexperovich commented Apr 17, 2018

Helix == MC

The telemetry tool sends telemetry to helix as if it was a job running on helix machines.

@chcosta
Copy link
Member

chcosta commented Apr 17, 2018

I thought "Helix" was the controller thing responsible for acquiring machines and scheduling jobs on them.

@alexperovich
Copy link
Member Author

alexperovich commented Apr 17, 2018

"Helix" is also the telemetry system that collects data from those jobs and provides them for MC to display. This telemetry tool hooks into that processing by sending its own telemetry. Specifically by calling the Telemetry apis.

@alexperovich
Copy link
Member Author

@tmat PTAL

@chcosta
Copy link
Member

chcosta commented Apr 17, 2018

I see. "Helix" still feels a bit overloaded. The namespace in this repo has thus far been "Microsoft.DotNet.Build.blah". I wonder if we should be slightly more targeted here than what's currently in the PR and go with something like "Microsoft.DotNet.Build.HelixTelemetry.Client".

@alexperovich
Copy link
Member Author

This client library will also be used by the other tasks brought in from build tools to send test jobs to helix. This isn't the "HelixTelemetry" client. Its the entire surface area of the helix api.

@chcosta
Copy link
Member

chcosta commented Apr 17, 2018

That makes more sense then.

@chcosta
Copy link
Member

chcosta commented Apr 17, 2018

I can't find this comment from @tmat in the PR

What are these files generated by? Can they be generated during build rather then checked in to the repo?

Was that addressed?

@alexperovich
Copy link
Member Author

They are generated by https://github.com/Azure/autorest. The config for the generator is in src/Microsoft.DotNet.Helix/Client/generator. Adding generation of the code to the build isn't really feasible because the generator is a nodejs process.

@@ -137,7 +137,7 @@ try {
}

if ($projects -eq "") {
$projects = Join-Path $RepoRoot "*.sln"
$projects = Join-Path $RepoRoot "**\*.sln"

This comment was marked as spam.

This comment was marked as spam.

@markwilkie
Copy link
Member

@alexperovich - please file an issue for you to provide a clear story for what the mechanics should be to update. Assuming this issue, I'm fine w/ you merging.

@alexperovich
Copy link
Member Author

Created #124

@alexperovich alexperovich merged commit 662c254 into dotnet:master Apr 17, 2018
@alexperovich alexperovich deleted the alperovi/addHelixApi branch May 22, 2018 22: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.

5 participants