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

Report the stats version pants is using to the server. #8086

Merged
merged 2 commits into from Jul 23, 2019

Conversation

@asherf
Copy link
Contributor

commented Jul 22, 2019

Problem

Pants doesn't report which version of stats it using when sending build stats data.

Solution

Put the stats version information in the HTTP request header.

Result

The server processing the build data doesn't have to infer which version of the stats it is getting.
The server can just read this information from the HTTP request headers.

Report the stats version pants is using to the server.
Signed-off-by: Asher Foa <1268088+asherf@users.noreply.github.com>

@asherf asherf force-pushed the asherf:headers branch from b1fab84 to 88c3551 Jul 22, 2019

@asherf

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2019

looks like I confused instance & class methods... derp.
fixing.

@asherf asherf force-pushed the asherf:headers branch from e3a6782 to abac382 Jul 22, 2019

@@ -367,12 +374,14 @@ def error(msg):
print(f'WARNING: Failed to upload stats to {stats_url} due to {msg}', file=sys.stderr)
return False

if stats_version not in cls.SUPPORTED_STATS_VERSIONS:

This comment has been minimized.

Copy link
@codealchemy

codealchemy Jul 22, 2019

Contributor

The version is already validated to be one of the choices on l91, so I don't believe this is necessary.

This comment has been minimized.

Copy link
@asherf

asherf Jul 22, 2019

Author Contributor

I think it is since post_stats() is a public method that can potentially be called from random places in the code.

This comment has been minimized.

Copy link
@stuhood

stuhood Jul 23, 2019

Member

So, while this is "public" from a python perspective, pants has its own concept of public APIs. If something is not marked :API: public as described here: https://www.pantsbuild.org/deprecation_policy.html , then you can assume that it cannot/should-not be used outside of github.com/pantsbuild/pants.

Pass stats_version to RunTracker.post_stats, have it validate it.
Signed-off-by: Asher Foa <1268088+asherf@users.noreply.github.com>

@asherf asherf force-pushed the asherf:headers branch from abac382 to b113a14 Jul 22, 2019

@asherf

This comment has been minimized.

Copy link
Contributor Author

commented Jul 23, 2019

I somewhat refactored this PR, So it will need another review.

@stuhood stuhood merged commit 570fab8 into pantsbuild:master Jul 23, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.