ocket8888 commented on code in PR #6639:
URL: https://github.com/apache/trafficcontrol/pull/6639#discussion_r846505645


##########
traffic_portal/app/src/common/api/CdniService.js:
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.
+ */
+
+var CdniService = function($http, ENV, messageModel) {
+
+       this.getCdniConfigRequests = function() {
+               return $http.get(ENV.api.unstable + 
'OC/CI/configuration/requests').then(
+                       function (result) {
+                               return result.data.response;
+                               },

Review Comment:
   this isn't something you need to change, but just for future reference our 
webpack version has been updated so we can use ES6 features in TP now. And in 
case that doesn't mean anything to you, it means we can use "fat arrow 
functions" to express these inline functions more succinctly with the syntax 
`(arg, list) => {some; statements;}`/`arg => {some; statements;}`/`(arg, list) 
=> result`/`arg=>result`.
   
   Like I said you don't have to change it, so I'm not gonna point out 
everywhere you could, but just to use this callback as an example, you could 
write the `getCdniConfigRequests` as such:
   ```javascript
   this.getCdniConfigRequests = () => 
$http.get(`${ENV.api.unstable}OC/CI/configuration/requests`).then(
       result => result.data.response,
       err => {
           messageModel.setMessages(err.data.alerts, true);
           throw err;
       }
   );
   ```
   which I like a lot more than having to write out all the boilerplate of 
`function` and `return` and `{`/`}` pairs each time.



##########
docs/source/api/v4/oc_fci_configuration_requests.rst:
##########
@@ -0,0 +1,96 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-oc-fci-configuration_requests:
+
+*********************************
+``OC/FCI/configuration/requests``
+*********************************
+
+``GET``
+=======
+Returns the requested updates for :abbr:`CDNi (Content Delivery Network 
Interconnect)` configurations. An optional ``id`` parameter will return only 
information for a specific request.
+
+:Auth. Required: Yes
+:Roles Required: "admin"
+:Permissions Required: CDNI-CAPACITY:ADMIN
+:Response Type:  Array
+
+Request Structure
+-----------------
+.. table:: Request Query Parameters
+
+       
+-----------+----------+---------------------------------------------------------------------------------------------------------------+
+       | Name      | Required | Description                                    
                                                               |
+       
+===========+==========+===============================================================================================================+
+       | id        | no       | Return only the configuration requests 
identified by this integral, unique identifier                         |
+       
+-----------+----------+---------------------------------------------------------------------------------------------------------------+
+
+Response Structure
+------------------
+:id:               An integral, unique identifier for the requested 
configuration updates.
+:ucdn:             The name of the :abbr:`uCDN (Upstream Content Delivery 
Network)` to which the requested changes apply.
+:data:             An array of generic :abbr:`FCI (Footprint and Capabilities 
Advertisement Interface)` base objects.
+:host:             The domain to which the requested changes apply.
+:request-type:     A string of the type of configuration update request.
+:async-status-id:  An integral, unique identifier for the associated 
asynchronous status.
+:capability-type:  A string of the type of base object.
+:capability-value: An array of the value for the base object.

Review Comment:
   I don't see `capability-type` or `capability-value` in the response example. 
Also these should be `camelCase` wherever the CDNi spec doesn't demand 
otherwise.



##########
traffic_ops/traffic_ops_golang/cdni/shared.go:
##########
@@ -738,32 +802,48 @@ const (
        CountryCode               = "countrycode"
 )
 
+// GenericHostMetadata contains the generic CDNi metadata for a requested 
update to a specific host.
 type GenericHostMetadata struct {
        Host         string           `json:"host"`
        HostMetadata HostMetadataList `json:"host-metadata"`
 }
 
+// GenericRequestMetadata contains the generic CDNi metadata for a requested 
update.
 type GenericRequestMetadata struct {
        Type     string          `json:"type"`
        Metadata json.RawMessage `json:"metadata"`
        Host     string          `json:"host,omitempty"`
 }
 
+// HostMetadataList contains CDNi metadata for a specific host.
 type HostMetadataList struct {
        Metadata json.RawMessage `json:"metadata"`
 }
 
+// GenericMetadata contains generic CDNi metadata.
 type GenericMetadata struct {
        Type  SupportedGenericMetadataType `json:"generic-metadata-type"`
        Value json.RawMessage              `json:"generic-metadata-value"`
 }
 
+// CapacityRequestedLimits contains the requested capacity limits.
 type CapacityRequestedLimits struct {
        RequestedLimits []CapacityLimit `json:"requested-limits"`
 }
 
+// CapacityLimit contains the limit information for a given footprint.
 type CapacityLimit struct {
        LimitType  string      `json:"limit-type"`
        LimitValue int64       `json:"limit-value"`
        Footprints []Footprint `json:"footprints"`
 }
+
+// ConfigurationUpdateRequest contains information about a requested CDNi 
configuration update request.
+type ConfigurationUpdateRequest struct {
+       ID            int             `json:"id"`
+       UCDN          string          `json:"ucdn"`
+       Data          json.RawMessage `json:"data"`
+       Host          string          `json:"host"`
+       RequestType   string          `json:"request-type" db:"request_type"`
+       AsyncStatusID int             `json:"async-status-id" 
db:"async_status_id"`

Review Comment:
   I don't know this for sure, but asynchronous job IDs doesn't seem like the 
kind of thing that would be in the CDI spec. If it isn't, then this should be 
camelCase in JSON as `asyncStatusID`



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to