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

Rewrote /deliveryservices/request to Go#3989

Merged
rawlinp merged 9 commits intoapache:masterfrom
ocket8888:dsrequests-go
Oct 21, 2019
Merged

Rewrote /deliveryservices/request to Go#3989
rawlinp merged 9 commits intoapache:masterfrom
ocket8888:dsrequests-go

Conversation

@ocket8888
Copy link
Copy Markdown
Contributor

@ocket8888 ocket8888 commented Oct 14, 2019

What does this PR (Pull Request) do?

Rewrites deliveryservices/request to Go.

Which Traffic Control components are affected by this PR?

  • Documentation
  • Traffic Ops

What is the best way to verify this PR?

Build the Traffic Ops RPM, install it somewhere with access to an SMTP server, configure it to use that server for sending mail, then request a Delivery Service via the deliveryservices/request endpoint.

This PR does not include tests, because there is currently no way to test functionality that depends on an external SMTP server.

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)

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 14, 2019

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

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 14, 2019

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

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 14, 2019

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

@ocket8888 ocket8888 added Traffic Ops related to Traffic Ops tech debt rework due to choosing easy/limited solution labels Oct 14, 2019
@ocket8888 ocket8888 added this to the Go Rewrite milestone Oct 14, 2019
@ocket8888 ocket8888 marked this pull request as ready for review October 14, 2019 17:26
@ocket8888 ocket8888 changed the title Dsrequests go Rewrote /deliveryservices/request to Go Oct 14, 2019
Copy link
Copy Markdown
Contributor

@mhoppa mhoppa left a comment

Choose a reason for hiding this comment

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

this PR looks good. I left a couple of comments but all minor/nit. Going to pull down to test prior to approval

Copy link
Copy Markdown
Contributor

@mhoppa mhoppa left a comment

Choose a reason for hiding this comment

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

some fields have the wrong type

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 17, 2019

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

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 18, 2019

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

@ocket8888
Copy link
Copy Markdown
Contributor Author

retest this please

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 18, 2019

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

@ocket8888
Copy link
Copy Markdown
Contributor Author

retest this please

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 18, 2019

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

Copy link
Copy Markdown
Contributor

@mhoppa mhoppa left a comment

Choose a reason for hiding this comment

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

The example post in the docs shouldnt be an invalid request as I am getting. FWIW all the bools are causing hasNegativeCachingCustomization: cannot be blank

Might be good to add some validation unit tests

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 21, 2019

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

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 21, 2019

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

@ocket8888
Copy link
Copy Markdown
Contributor Author

The request example in the docs isn't invalid - I accidentally changed some fields to strings that shouldn't have been when I was making changes you requested above.

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 21, 2019

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

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 21, 2019

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

Copy link
Copy Markdown
Contributor

@mhoppa mhoppa left a comment

Choose a reason for hiding this comment

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

Tested locally and works as expected.

@ocket8888
Copy link
Copy Markdown
Contributor Author

retest this please

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 21, 2019

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

@rawlinp rawlinp self-assigned this Oct 21, 2019
@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Oct 21, 2019

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

@rawlinp rawlinp merged commit 0974c74 into apache:master Oct 21, 2019
@ocket8888 ocket8888 deleted the dsrequests-go branch October 21, 2019 20:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tech debt rework due to choosing easy/limited solution Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rewrite /deliveryservices/request to Go

5 participants