rawlinp commented on a change in pull request #4734:
URL: https://github.com/apache/trafficcontrol/pull/4734#discussion_r432610548
##########
File path: traffic_ops/traffic_ops_golang/steeringtargets/steeringtargets.go
##########
@@ -159,7 +160,33 @@ func read(tx *sqlx.Tx, parameters map[string]string, user
*auth.CurrentUser) ([]
return filteredTargets, nil, nil, http.StatusOK
}
+func checkTypeValidity(typeID int, tx *sqlx.Tx) (bool, error) {
+ row := tx.QueryRow(`SELECT name FROM type WHERE id=$1`, typeID)
Review comment:
It might be better to check that the `use_in_table` value is
`steering_target` for the given type id, instead of checking against the 4 we
have currently (in case we add more in the future).
##########
File path: docs/source/api/v3/steering_id_targets.rst
##########
@@ -72,9 +72,9 @@ Response Structure
:deliveryServiceId: An integral, unique identifier for the steering
:term:`Delivery Service`
:target: A string that is the :ref:`ds-xmlid` of this target
:term:`Delivery Service`
:targetId: An integral, unique identifier for this target
:term:`Delivery Service`
-:type: The routing type of this target :term:`Delivery Service`
+:type: The routing type of this target :term:`Delivery Service`.
This should be one of ``STEERING_WEIGHT``, ``STEERING_ORDER``,
``STEERING_GEO_ORDER`` or ``STEERING_GEO_WEIGHT``
Review comment:
~routing type~ steering type
##########
File path: docs/source/api/v3/steering_id_targets.rst
##########
@@ -122,8 +122,8 @@ Request Structure
+------+---------------------------------------------------------------------------------------------------------+
:targetId: The integral, unique identifier of a :term:`Delivery Service` which
shall be a new steering target for the :term:`Delivery Service` identified by
the ``ID`` path parameter
-:typeId: The integral, unique identifier of the routing type of the new
target :term:`Delivery Service`
-:value: The 'weight' which shall be attributed to the new target
:term:`Delivery Service`
+:typeId: The integral, unique identifier of the routing type of the new
target :term:`Delivery Service`. This should be corresponding to one of
``STEERING_WEIGHT``, ``STEERING_ORDER``, ``STEERING_GEO_ORDER`` or
``STEERING_GEO_WEIGHT``
Review comment:
~routing type~ steering type
##########
File path: CHANGELOG.md
##########
@@ -360,3 +361,8 @@ The format is based on [Keep a
Changelog](http://keepachangelog.com/en/1.0.0/).
[4.0.0]:
https://github.com/apache/trafficcontrol/compare/RELEASE-3.0.0...RELEASE-4.0.0
[3.0.0]:
https://github.com/apache/trafficcontrol/compare/RELEASE-2.2.0...RELEASE-3.0.0
[2.2.0]:
https://github.com/apache/trafficcontrol/compare/RELEASE-2.1.0...RELEASE-2.2.0
+
+
+[]: https://github.com/apache/trafficcontrol/issues/4116
Review comment:
are these necessary?
##########
File path: docs/source/api/v3/steering_id_targets_targetID.rst
##########
@@ -39,8 +39,8 @@ Request Structure
| targetID | The integral, unique identifier of a :term:`Delivery
Service` which is a target of the :term:`Delivery Service` identified by ``ID``
|
+----------+--------------------------------------------------------------------------------------------------------------------------------------+
-:typeId: The integral, unique identifier of the :ref:`routing type <ds-types>`
of the target :term:`Delivery Service`
-:value: The 'weight' which shall be attributed to the target :term:`Delivery
Service`
+:typeId: The integral, unique identifier of the :ref:`routing type <ds-types>`
of the target :term:`Delivery Service`. This should be corresponding to one of
``STEERING_WEIGHT``, ``STEERING_ORDER``, ``STEERING_GEO_ORDER`` or
``STEERING_GEO_WEIGHT``
Review comment:
~routing type~ steering type, and that `:ref:` likely needs removed or
changed to reference a new `<steering-types>` ref
##########
File path:
traffic_ops/traffic_ops_golang/steeringtargets/steeringtargets_test.go
##########
@@ -0,0 +1,89 @@
+package steeringtargets
+
+import (
+ "github.com/apache/trafficcontrol/lib/go-tc"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+ "github.com/jmoiron/sqlx"
+ "gopkg.in/DATA-DOG/go-sqlmock.v1"
+ "net/http"
+ "testing"
+)
+
+/*
Review comment:
In general I think the Apache header goes _after_ the package
declaration but _before_ the imports.
##########
File path: docs/source/api/v3/steering_id_targets.rst
##########
@@ -148,9 +148,9 @@ Response Structure
:deliveryServiceId: An integral, unique identifier for the steering
:term:`Delivery Service`
:target: A string that is the :ref:`ds-xmlid` of this target
:term:`Delivery Service`
:targetId: An integral, unique identifier for this target
:term:`Delivery Service`
-:type: The routing type of this target :term:`Delivery Service`
+:type: The routing type of this target :term:`Delivery Service`.
This should be one of ``STEERING_WEIGHT``, ``STEERING_ORDER``,
``STEERING_GEO_ORDER`` or ``STEERING_GEO_WEIGHT``
Review comment:
~routing type~ steering type
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
