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

Deliveryservice Topologies#4692

Merged
ocket8888 merged 21 commits intoapache:masterfrom
rawlinp:deliveryservice-topologies
May 27, 2020
Merged

Deliveryservice Topologies#4692
ocket8888 merged 21 commits intoapache:masterfrom
rawlinp:deliveryservice-topologies

Conversation

@rawlinp
Copy link
Copy Markdown
Contributor

@rawlinp rawlinp commented May 14, 2020

What does this PR (Pull Request) do?

Add a new Topology field to Delivery Services in TO API 3.0 in order to assign Delivery Services to Topologies.

Which Traffic Control components are affected by this PR?

  • Documentation
  • Traffic Control Client (Go)
  • Traffic Ops
  • Traffic Portal

What is the best way to verify this PR?

  1. Run the unit tests, verify they pass
  2. Run the TO API tests, verify they pass

The following criteria are ALL met by this PR

  • This PR includes tests
  • This PR includes documentation
  • This PR includes an update to CHANGELOG.md
  • This PR includes any and all required license headers
  • 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)

@rawlinp rawlinp added new feature A new feature, capability or behavior Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1 documentation related to documentation TO Client (Go) related to the Go implementation of a TC client WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) labels May 14, 2020
@rawlinp rawlinp force-pushed the deliveryservice-topologies branch from adb1f6c to bacb7a1 Compare May 15, 2020 14:40
@rawlinp rawlinp added WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) and removed WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) labels May 15, 2020
@rawlinp rawlinp removed the WIP "Work-in-Progress" - do not merge! (use 'draft' pull requests from now on) label May 15, 2020
Comment thread traffic_ops/testing/api/v3/user_test.go Outdated
Comment thread traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go Outdated
Comment thread traffic_ops/traffic_ops_golang/deliveryservice/request/requests_test.go Outdated
Comment thread docs/source/api/v3/deliveryservices.rst Outdated
Copy link
Copy Markdown
Member

@zrhoffman zrhoffman left a comment

Choose a reason for hiding this comment

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

TO API tests pass, TO unit tests pass, gofmt does not change the formatting of files changed in this PR, and the docs build without warnings related to this PR and look good.

The Traffic Portal portion of this PR also needs to be reviewed.

Copy link
Copy Markdown
Contributor

@ocket8888 ocket8888 left a comment

Choose a reason for hiding this comment

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

I left all of my DS form comments on the DNS template - but they (probably mostly) apply to all of them

Comment thread traffic_portal/app/src/common/api/ServerService.js Outdated
Comment thread traffic_portal/app/src/common/api/ServerService.js Outdated
<li role="menuitem"><a ng-click="viewCharts()">View Charts</a></li>
<hr class="divider"/>
<li role="menuitem"><a ng-click="viewServers()">Manage Servers</a></li>
<li ng-if="::(!deliveryService.topology)" role="menuitem"><a ng-click="viewServers()">Manage Servers</a></li>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO these should be !(deliveryservice.Topology instanceof Object)

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.

Nah

const a = {topology: "test"};
console.log(a.topology instanceof Object); // false

Copy link
Copy Markdown
Member

@zrhoffman zrhoffman May 26, 2020

Choose a reason for hiding this comment

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

Oh, that's just the name. typeof deliveryService.Topology !== 'string' in that case

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.

But then you could have

const a = {topology: ""};
console.log(typeof a.topology === "string"); //true

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

True, the loose comparison is useful in this case.

@ocket8888 ocket8888 merged commit fac8490 into apache:master May 27, 2020
@rawlinp rawlinp deleted the deliveryservice-topologies branch May 27, 2020 16:51
@zrhoffman zrhoffman added this to the Flexible Topologies milestone Sep 18, 2020
@zrhoffman zrhoffman removed this from the Flexible Topologies milestone Oct 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

documentation related to documentation 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 Traffic Portal v1 related to Traffic Portal version 1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update Delivery Service APIs with new topology field

5 participants