ocket8888 commented on code in PR #7111: URL: https://github.com/apache/trafficcontrol/pull/7111#discussion_r989135302
########## CHANGELOG.md: ########## @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - [#2101](https://github.com/apache/trafficcontrol/issues/2101) *Traffic Portal* Added the ability to tell if a Delivery Service is the target of another steering DS. - [#6033](https://github.com/apache/trafficcontrol/issues/6033) *Traffic Ops, Traffic Portal* Added ability to assign multiple server capabilities to a server. - [#7032](https://github.com/apache/trafficcontrol/issues/7032) *Cache Config* Add t3c-apply flag to use local ATS version for config generation rather than Server package Parameter, to allow managing the ATS OS package via external tools. See 'man t3c-apply' and 'man t3c-generate' for details. +- [#7097](https://github.com/apache/trafficcontrol/issues/7097) *Traffic Ops* Added the `regional` field, which indicates whether `maxOriginConnections` should be per Cache Group Review Comment: > Added the `regional` field, which ... Added to what? ########## docs/source/overview/delivery_services.rst: ########## @@ -505,6 +505,9 @@ Max Origin Connections ---------------------- The maximum number of TCP connections individual :term:`Mid-tier cache servers` are allowed to make to the `Origin Server Base URL`. A value of ``0`` in this field indicates that there is no maximum. + + .. note:: Max Origin Connections can be made per-:ref:`Cache Group <cache-groups>` by setting the :ref:`ds-regional` field. Review Comment: indentation here is causing this note to be rendered as a block quote - it's not a quote, so it shouldn't be indented. ########## traffic_portal/app/src/common/modules/form/deliveryService/form.deliveryService.DNS.tpl.html: ########## @@ -475,6 +475,26 @@ <h3 ng-if="!open()">Previous Value</h3> </aside> </div> </div> + <div class="form-group" ng-class="{'has-error': hasError(routingConfig.regional), 'has-feedback': hasError(routingConfig.regional)}"> + <label class="has-tooltip control-label col-md-2 col-sm-2 col-xs-12" for="regional">Regional Max Origin Connections *<div class="helptooltip"> + <div class="helptext"> + If your Delivery Service is Regional, then you only expect it to be used by a single Cache Group. Enable this to make Max Origin Connections per Cache Group instead of a value used over all last-tier Cache Groups. <a href="https://traffic-control-cdn.readthedocs.io/en/latest/overview/delivery_services.html#regional"; target="_blank">See Regional Delivery Services</a> + </div> + </div> + </label> + <div class="col-md-10 col-sm-10 col-xs-12"> + <select id="regional" name="regional" class="form-control" ng-model="deliveryService.regional" required> + <option ng-value="true">Enabled</option> + <option ng-value="false" selected>Disabled</option> + </select> Review Comment: this is pretty standard throughout TP, so you can leave it like this if you want, but for my money boolean inputs should use `input[type="checkbox"]` because that's what it's for. ########## traffic_ops/app/db/migrations/2022100210472946_ds_regional.up.sql: ########## @@ -0,0 +1,30 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +ALTER TABLE public.deliveryservice +ADD COLUMN regional BOOLEAN NOT NULL DEFAULT FALSE; + +/* Set `regional` to `false` if it does not exist */ +UPDATE public.deliveryservice_request +SET deliveryservice = deliveryservice || '{"regional": false}' +WHERE deliveryservice->>'regional' IS NULL; + +/* Set `regional` to `false` it does not exist and `original` is not null */ +UPDATE public.deliveryservice_request +SET original = original || '{"regional": false}' +WHERE original IS NOT NULL Review Comment: why check if original is `NULL` if not doing the same for deliveryservice? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
