Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Rewrite stats summary post to golang#4173

Merged
ocket8888 merged 24 commits into
apache:masterfrom
mhoppa:feature/rewrite_stats_summary_post
Jan 29, 2020
Merged

Rewrite stats summary post to golang#4173
ocket8888 merged 24 commits into
apache:masterfrom
mhoppa:feature/rewrite_stats_summary_post

Conversation

@mhoppa

@mhoppa mhoppa commented Dec 6, 2019

Copy link
Copy Markdown
Contributor

What does this PR (Pull Request) do?

  • This PR does not have a related issue

Rewrite /stats_summary/create into a new route of /stats_summary

Which Traffic Control components are affected by this PR?

  • Documentation
  • Traffic Control Client
  • Traffic Ops
  • Traffic Stats

What is the best way to verify this PR?

If this is a bug fix, what versions of Traffic Control are affected?

The following criteria are ALL met by this PR

  • This PR includes tests OR I have explained why tests are unnecessary
  • This PR includes documentation OR I have explained why documentation is unnecessary
  • This PR includes an update to CHANGELOG.md OR such an update is not necessary
  • This PR includes any and all required license headers
  • This PR ensures that database migration sequence is correct OR this PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

Additional Information

@asf-ci

asf-ci commented Dec 6, 2019

Copy link
Copy Markdown
Contributor

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4883/

@ocket8888 ocket8888 added this to the Go Rewrite milestone Dec 6, 2019
@ocket8888 ocket8888 added tech debt rework due to choosing easy/limited solution Traffic Ops related to Traffic Ops WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) labels Dec 6, 2019
@mhoppa
mhoppa marked this pull request as ready for review January 6, 2020 17:57
@mhoppa mhoppa changed the title WIP - Rewrite stats summary post to golang Rewrite stats summary post to golang Jan 6, 2020
Comment thread docs/source/api/stats_summary.rst Outdated
Comment thread docs/source/api/stats_summary.rst
Comment thread docs/source/api/stats_summary.rst Outdated
Comment thread traffic_ops/client/stats_summary.go
@ocket8888 ocket8888 removed the WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) label Jan 13, 2020
Comment thread docs/source/api/stats_summary.rst Outdated
@mhoppa
mhoppa requested a review from ocket8888 January 29, 2020 16:05
@mhoppa

mhoppa commented Jan 29, 2020

Copy link
Copy Markdown
Contributor Author

@ocket8888 once you get a chance can you re review?? :)

@ocket8888 ocket8888 left a comment

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.

All of the actual code and docs changes look fine, but if this is going to close #3833 then it needs to either rewrite or deprecate summary_stats/create (and I'm pretty sure we want the latter). Which would mean a patch in the Perl handler to return a deprecation notice and a line in the CHANGELOG. If you wanna do that in a separate PR that's fine, but then this doesn't close #3833.

Comment thread CHANGELOG.md
@mhoppa

mhoppa commented Jan 29, 2020

Copy link
Copy Markdown
Contributor Author

#4186 already posted waiting for this to merge let me remove the issue from the pull request template so it doesnt close it

@ocket8888 ocket8888 left a comment

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.

Manual tests confirm it works according to the docs which build without errors or warnings and look fine, all unit and api/client integration tests passing.

@ocket8888
ocket8888 merged commit ae5ae2f into apache:master Jan 29, 2020
@ocket8888 ocket8888 removed this from the Go Rewrite milestone Jan 29, 2020
@ocket8888 ocket8888 added documentation related to documentation improvement The functionality exists but it could be improved in some way. medium impact impacts a significant portion of a CDN, or has the potential to do so tests related to tests and/or testing infrastructure TO Client (Go) related to the Go implementation of a TC client and removed tech debt rework due to choosing easy/limited solution labels Jan 29, 2020
@mhoppa
mhoppa deleted the feature/rewrite_stats_summary_post branch January 29, 2020 17:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

documentation related to documentation improvement The functionality exists but it could be improved in some way. medium impact impacts a significant portion of a CDN, or has the potential to do so tests related to tests and/or testing infrastructure TO Client (Go) related to the Go implementation of a TC client Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants