zrhoffman commented on code in PR #7459:
URL: https://github.com/apache/trafficcontrol/pull/7459#discussion_r1187929921


##########
experimental/traffic-portal/src/app/api/cdn.service.ts:
##########
@@ -109,4 +109,16 @@ export class CDNService extends APIService {
 
                return this.put<ResponseCDN>(`cdns/${id}`, body).toPromise();
        }
+
+       /**
+        * Queues or dequeues updates on a CDN's servers.
+        *
+        * @param cdn The CDN to queue or dequeue updates on.
+        * @param action The action to perform on the CDN, either "queue" or 
"dequeue".
+        */
+       public async queueCDNUpdates(cdn: ResponseCDN, action: "queue" | 
"dequeue"): Promise<CDNQueueResponse> {

Review Comment:
   I know what you mean, but accepting both and checking for either both ID and 
`ResponseCDN` instance in all places is not the way to do it, IMO, because it 
is extra work, and the point of Typescript is to eliminate ambiguities like 
that one.
   
   Maybe a future improvement having it take a new `CDN` type that *can* have 
as much info as `ResponseCDN` but only requires an `ID`, and it's only resolved 
by `CDNService` before making an API call (or some other way to defer its 
resolution)?
   
   If something like that is acceptable, then IMO we should do that in the 
places we're currently accepting either an object or ID.



-- 
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