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

Add TO client ATS config funcs#2987

Closed
rob05c wants to merge 3 commits intoapache:masterfrom
rob05c:to-client-atsconfigs
Closed

Add TO client ATS config funcs#2987
rob05c wants to merge 3 commits intoapache:masterfrom
rob05c:to-client-atsconfigs

Conversation

@rob05c
Copy link
Copy Markdown
Member

@rob05c rob05c commented Nov 2, 2018

What does this PR do?

Add TO client ATS config funcs

Which TC components are affected by this PR?

  • Documentation
  • Grove
  • Traffic Analytics
  • Traffic Monitor
  • Traffic Ops
  • Traffic Ops ORT
  • Traffic Portal
  • Traffic Router
  • Traffic Stats
  • Traffic Vault
  • Other _________

What is the best way to verify this PR?

Check all that apply

  • This PR includes tests
  • This PR includes documentation updates
  • This PR includes an update to CHANGELOG.md
  • This PR includes all required license headers
  • This PR includes a database migration (ensure that migration sequence is correct)
  • This PR fixes a serious security flaw. Read more: www.apache.org/security

@rob05c rob05c added new feature A new feature, capability or behavior Traffic Ops Client labels Nov 2, 2018
@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Nov 2, 2018

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

@rob05c rob05c force-pushed the to-client-atsconfigs branch 2 times, most recently from 02804ec to a0fbf84 Compare November 2, 2018 20:18
@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Nov 2, 2018

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

@rob05c rob05c force-pushed the to-client-atsconfigs branch from a0fbf84 to d360641 Compare November 2, 2018 20:27
@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Nov 2, 2018

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

@dangogh
Copy link
Copy Markdown
Member

dangogh commented Nov 8, 2018

retest this please

@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Nov 8, 2018

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

dangogh
dangogh suggested changes Nov 8, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@dangogh dangogh left a comment

Choose a reason for hiding this comment

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

missing Apache license header in ats.go. Have yet to try to build it..

@rob05c
Copy link
Copy Markdown
Member Author

rob05c commented Nov 8, 2018

@dangogh Fixed

@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Nov 8, 2018

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

@rob05c rob05c force-pushed the to-client-atsconfigs branch from 961f8f0 to 87ac905 Compare December 3, 2018 21:40
@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Dec 3, 2018

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

moltzaum
moltzaum suggested changes Dec 6, 2018
View reviewed changes
Comment thread traffic_ops/testing/api/v14/atsconfig_test.go Outdated
Comment thread traffic_ops/client/atsconfig.go Outdated
Comment thread traffic_ops/client/atsconfig.go Outdated
@moltzaum
Copy link
Copy Markdown
Contributor

moltzaum commented Dec 6, 2018 •
edited
Loading

I tested the PR by running the test TestATSConfigs. Every public-facing client function is run inside the test and they didn't return any errors when I ran them. Not sure if trying to make them fail makes sense in this review.

Having this pulled in would be helpful for ATS config tests that depend on it. The client is common to all the config tests.

@rob05c rob05c force-pushed the to-client-atsconfigs branch from 87ac905 to e8e1cfc Compare December 6, 2018 22:10
@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Dec 6, 2018

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

@moltzaum
Copy link
Copy Markdown
Contributor

moltzaum commented Dec 13, 2018

Where is this at? @rob05c do you plan on adding more? @dangogh in order to merge do you want to see anything else?

@rob05c
Copy link
Copy Markdown
Member Author

rob05c commented Dec 13, 2018 •
edited
Loading

Where is this at?

I think it's ready to be merged. I fixed your commented code comment, and commented on the capitalization comment.

I don't object to using the get helper instead of request, but it'll take time to implement and verify. Personally I don't think it's big enough that it should prevent merging. But I'll try to find time, if this isn't merged shortly.

@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Dec 13, 2018

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

@rob05c rob05c force-pushed the to-client-atsconfigs branch from a9abdfa to 8a271a9 Compare February 12, 2019 21:17
@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Feb 12, 2019

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

@mitchell852 mitchell852 added TO Client (Go) related to the Go implementation of a TC client TC Client (python) related to the Python implementation of a TC client and removed Traffic Ops Client labels Jun 20, 2019
@rob05c rob05c removed the TC Client (python) related to the Python implementation of a TC client label Aug 1, 2019
@mitchell852 mitchell852 added the Traffic Ops related to Traffic Ops label Nov 18, 2019
@ocket8888
Copy link
Copy Markdown
Contributor

ocket8888 commented Nov 10, 2021

APIv1.4 no longer exists, and the API no longer serves configuration files at all - should this PR be closed?

@rob05c rob05c closed this Nov 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@dangogh dangogh dangogh requested changes

@moltzaum moltzaum moltzaum requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

new feature A new feature, capability or behavior TO Client (Go) related to the Go implementation of a TC client Traffic Ops related to Traffic Ops

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@rob05c @asfgit @dangogh @moltzaum @ocket8888 @mitchell852