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

Remove /servers/details from APIv4 #6819

Merged
merged 6 commits into from
May 17, 2022

Conversation

ocket8888
Copy link
Contributor

@ocket8888 ocket8888 commented May 10, 2022

This PR removes /servers/details from version 4.0 of the API, and adds deprecation notices to v3.


Which Traffic Control components are affected by this PR?

  • Traffic Control Clients (both Go and Python)
  • Traffic Ops

What is the best way to verify this PR?

Make sure all the tests still pass, verify endpoint doesn't exist in v4, returns deprecation alerts in lower versions

PR submission checklist

  • This PR has tests
  • This PR has documentation
  • This PR has a CHANGELOG.md entry
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@ocket8888 ocket8888 added Traffic Ops related to Traffic Ops tech debt rework due to choosing easy/limited solution TC Client (python) related to the Python implementation of a TC client TO Client (Go) related to the Go implementation of a TC client labels May 10, 2022
@ocket8888 ocket8888 force-pushed the to/remove-servers-details branch from f30d4cb to aa7194b Compare May 11, 2022 04:40
@ocket8888 ocket8888 force-pushed the to/remove-servers-details branch from aa7194b to b524bc8 Compare May 13, 2022 21:34
@rimashah25
Copy link
Contributor

Curl cmd to servers/details API v3.0 with query param hostName now works with the latest change commit and correct warning.

Curl cmd to servers/details API v3.0 with query param physLocationID is working fine with correct warning.

Curl cmd to servers/details API v4.0 show appropriate error.

@rimashah25
Copy link
Contributor

APIv3 integration are failing on local as well as github.

@rimashah25
Copy link
Contributor

I also fixed #6800 while I was in the code anyway.

Is this valid?

@ocket8888
Copy link
Contributor Author

I also fixed #6800 while I was in the code anyway.

Is this valid?

It was, it's just that it's already been fixed. I can remove it from the description, though, since it's not technically true anymore.

@ocket8888
Copy link
Contributor Author

APIv3 integration are failing on local as well as github.

v2 tests are failing too, but I didn't want to fix those because I was hoping the PR to remove APIv2 would be opened soon and I could just rely on that instead of fixing anything. I'm re-running v3 because they tend to be flaky, but if they still fail I'll probably just fix both.

@rimashah25
Copy link
Contributor

APIv3 integration are failing on local as well as github.

v2 tests are failing too, but I didn't want to fix those because I was hoping the PR to remove APIv2 would be opened soon and I could just rely on that instead of fixing anything. I'm re-running v3 because they tend to be flaky, but if they still fail I'll probably just fix both.

I am ok with v2 failure since they will be removed soon. But v3 is still failing, so we need to fix that before we merge the PR.

Copy link
Contributor

@rimashah25 rimashah25 left a comment

Choose a reason for hiding this comment

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

LGTM

@shamrickus shamrickus merged commit b9cdfd5 into apache:master May 17, 2022
@ocket8888 ocket8888 deleted the to/remove-servers-details branch May 17, 2022 22:22
@asf-ci asf-ci mentioned this pull request Jun 1, 2022
4 tasks
zrhoffman pushed a commit to zrhoffman/trafficcontrol that referenced this pull request Oct 2, 2022
* Add deprecation notices, remove v4 handling, fix apache#6800

* Update documentation - add deprecation notices to v3, remove from v4

* Update clients - remove from v4, add deprecation notices to v3

* Update CHANGELOG

* fix unable to get server details by hostname if physloc not also provided

* Fix embedded 'response' object
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TC Client (python) related to the Python implementation of a TC client tech debt rework due to choosing easy/limited solution 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