You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2021/09/02 15:39:37 UTC

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6154: Add support for cdn and maxRevalDurationDays qparams to GET /jobs

ocket8888 commented on a change in pull request #6154:
URL: https://github.com/apache/trafficcontrol/pull/6154#discussion_r701198873



##########
File path: docs/source/api/v2/jobs.rst
##########
@@ -31,23 +31,27 @@ Request Structure
 -----------------
 .. table:: Request Query Parameters
 
-	+-----------------+----------+----------------------------------------------------------------------------------------------------------------------+
-	| Name            | Required | Description                                                                                                          |
-	+=================+==========+======================================================================================================================+
-	| assetUrl        | no       | Return only invalidation jobs that operate on URLs by matching this regular expression                               |
-	+-----------------+----------+----------------------------------------------------------------------------------------------------------------------+
-	| createdBy       | no       | Return only invalidation jobs that were created by the user with this username                                       |
-	+-----------------+----------+----------------------------------------------------------------------------------------------------------------------+
-	| deliveryService | no       | Return only invalidation jobs that operate on the :term:`Delivery Service` with this :ref:`ds-xmlid`                 |
-	+-----------------+----------+----------------------------------------------------------------------------------------------------------------------+
-	| dsId            | no       | Return only invalidation jobs pending on the :term:`Delivery Service` identified by this integral, unique identifier |
-	+-----------------+----------+----------------------------------------------------------------------------------------------------------------------+
-	| id              | no       | Return only the single invalidation job identified by this integral, unique identifer                                |
-	+-----------------+----------+----------------------------------------------------------------------------------------------------------------------+
-	| keyword         | no       | Return only invalidation jobs that have this "keyword" - only "PURGE" should exist                                   |
-	+-----------------+----------+----------------------------------------------------------------------------------------------------------------------+
-	| userId          | no       | Return only invalidation jobs created by the user identified by this integral, unique identifier                     |
-	+-----------------+----------+----------------------------------------------------------------------------------------------------------------------+
+	+----------------------+----------+------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+	| Name                 | Required | Description                                                                                                                                                      |
+	+======================+==========+==================================================================================================================================================================+
+	| assetUrl             | no       | Return only invalidation jobs that operate on URLs by matching this regular expression                                                                           |
+	+----------------------+----------+------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+	| cdn                  | no       | Return only invalidation jobs for delivery services with this CDN name                                                                                           |
+	+----------------------+----------+------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+	| createdBy            | no       | Return only invalidation jobs that were created by the user with this username                                                                                   |
+	+----------------------+----------+------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+	| deliveryService      | no       | Return only invalidation jobs that operate on the :term:`Delivery Service` with this :ref:`ds-xmlid`                                                             |
+	+----------------------+----------+------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+	| dsId                 | no       | Return only invalidation jobs pending on the :term:`Delivery Service` identified by this integral, unique identifier                                             |
+	+----------------------+----------+------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+	| id                   | no       | Return only the single invalidation job identified by this integral, unique identifer                                                                            |
+	+----------------------+----------+------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+	| keyword              | no       | Return only invalidation jobs that have this "keyword" - only "PURGE" should exist                                                                               |
+	+----------------------+----------+------------------------------------------------------------------------------------------------------------------------------------------------------------------+
+	| maxRevalDurationDays | no       | Return only invalidation jobs with a start_time that is within the window defined by the ``maxRevalDurationDays`` :term:`Parameter` in :ref:`the-global-profile` |

Review comment:
       The field returned by the API is `startTime` - `start_time` is an internal DB column name with which consumers of the API need not concern themselves.

##########
File path: traffic_ops/testing/api/v4/jobs_test.go
##########
@@ -482,6 +482,62 @@ func GetTestJobsByValidData(t *testing.T) {
 	} else {
 		t.Error("No user response available for get user by name")
 	}
+
+	// get maxRevalDurationDays parameter value
+	opts = client.NewRequestOptions()
+	opts.QueryParameters.Set("name", "maxRevalDurationDays")
+	params, _, err := TOSession.GetParameters(opts)
+	if err != nil {
+		t.Fatalf("unexpected error getting maxRevalDurationDays parameter")
+	}
+	if len(params.Response) != 1 {
+		t.Fatalf("expected 1 maxRevalDurationDays parameter, got %d", len(params.Response))
+	}
+	maxRevalDurationDays, err := strconv.Atoi(params.Response[0].Value)
+	if err != nil {
+		t.Fatalf("unexpected error converting maxRevalDurationDays value to int: %v", err)
+	}
+
+	// get jobs created within maxRevalDurationDays window
+	opts = client.NewRequestOptions()
+	opts.QueryParameters.Set("maxRevalDurationDays", "")
+	maxRevalJobs, _, err := TOSession.GetInvalidationJobs(opts)
+	if err != nil {
+		t.Errorf("unexpected error getting jobs by maxRevalDurationDays: %v", err)
+	} else if len(maxRevalJobs.Response) < 1 {
+		t.Errorf("GET /jobs by maxRevalDurationDays - expected at least 1 job")
+	}
+	for _, j := range maxRevalJobs.Response {
+		if time.Since((*j.StartTime).Time) > time.Duration(maxRevalDurationDays)*24*time.Hour {

Review comment:
       This line will segfault if the API returns a malformed response with a null or undefined `startTime`

##########
File path: traffic_ops/testing/api/v4/jobs_test.go
##########
@@ -482,6 +482,62 @@ func GetTestJobsByValidData(t *testing.T) {
 	} else {
 		t.Error("No user response available for get user by name")
 	}
+
+	// get maxRevalDurationDays parameter value
+	opts = client.NewRequestOptions()
+	opts.QueryParameters.Set("name", "maxRevalDurationDays")
+	params, _, err := TOSession.GetParameters(opts)
+	if err != nil {
+		t.Fatalf("unexpected error getting maxRevalDurationDays parameter")
+	}
+	if len(params.Response) != 1 {
+		t.Fatalf("expected 1 maxRevalDurationDays parameter, got %d", len(params.Response))
+	}
+	maxRevalDurationDays, err := strconv.Atoi(params.Response[0].Value)
+	if err != nil {
+		t.Fatalf("unexpected error converting maxRevalDurationDays value to int: %v", err)
+	}
+
+	// get jobs created within maxRevalDurationDays window
+	opts = client.NewRequestOptions()
+	opts.QueryParameters.Set("maxRevalDurationDays", "")
+	maxRevalJobs, _, err := TOSession.GetInvalidationJobs(opts)
+	if err != nil {
+		t.Errorf("unexpected error getting jobs by maxRevalDurationDays: %v", err)
+	} else if len(maxRevalJobs.Response) < 1 {
+		t.Errorf("GET /jobs by maxRevalDurationDays - expected at least 1 job")
+	}
+	for _, j := range maxRevalJobs.Response {
+		if time.Since((*j.StartTime).Time) > time.Duration(maxRevalDurationDays)*24*time.Hour {
+			t.Errorf("GET /jobs by maxRevalDurationDays returned job that is older than %d days: {%s, %s, %v}", maxRevalDurationDays, *j.DeliveryService, *j.AssetURL, *j.StartTime)
+		}
+	}
+
+	// create DS xml_id -> cdn_id lookup map
+	dses, _, err := TOSession.GetDeliveryServices(client.NewRequestOptions())
+	if err != nil {
+		t.Fatalf("unexpectd error getting delivery services: %v", err)
+	}
+	dsToCDN := make(map[string]string, len(dses.Response))
+	for _, ds := range dses.Response {
+		dsToCDN[*ds.XMLID] = *ds.CDNName

Review comment:
       This line will segfault if the API returns a malformed response with null or undefined `xmlId` and/or `cdnName` 

##########
File path: traffic_ops/testing/api/v4/jobs_test.go
##########
@@ -482,6 +482,62 @@ func GetTestJobsByValidData(t *testing.T) {
 	} else {
 		t.Error("No user response available for get user by name")
 	}
+
+	// get maxRevalDurationDays parameter value
+	opts = client.NewRequestOptions()
+	opts.QueryParameters.Set("name", "maxRevalDurationDays")
+	params, _, err := TOSession.GetParameters(opts)
+	if err != nil {
+		t.Fatalf("unexpected error getting maxRevalDurationDays parameter")

Review comment:
       This should print the error that occurred

##########
File path: traffic_ops/testing/api/v4/jobs_test.go
##########
@@ -482,6 +482,62 @@ func GetTestJobsByValidData(t *testing.T) {
 	} else {
 		t.Error("No user response available for get user by name")
 	}
+
+	// get maxRevalDurationDays parameter value
+	opts = client.NewRequestOptions()
+	opts.QueryParameters.Set("name", "maxRevalDurationDays")
+	params, _, err := TOSession.GetParameters(opts)
+	if err != nil {
+		t.Fatalf("unexpected error getting maxRevalDurationDays parameter")
+	}
+	if len(params.Response) != 1 {
+		t.Fatalf("expected 1 maxRevalDurationDays parameter, got %d", len(params.Response))
+	}
+	maxRevalDurationDays, err := strconv.Atoi(params.Response[0].Value)
+	if err != nil {
+		t.Fatalf("unexpected error converting maxRevalDurationDays value to int: %v", err)
+	}
+
+	// get jobs created within maxRevalDurationDays window
+	opts = client.NewRequestOptions()
+	opts.QueryParameters.Set("maxRevalDurationDays", "")
+	maxRevalJobs, _, err := TOSession.GetInvalidationJobs(opts)
+	if err != nil {
+		t.Errorf("unexpected error getting jobs by maxRevalDurationDays: %v", err)
+	} else if len(maxRevalJobs.Response) < 1 {
+		t.Errorf("GET /jobs by maxRevalDurationDays - expected at least 1 job")
+	}
+	for _, j := range maxRevalJobs.Response {
+		if time.Since((*j.StartTime).Time) > time.Duration(maxRevalDurationDays)*24*time.Hour {
+			t.Errorf("GET /jobs by maxRevalDurationDays returned job that is older than %d days: {%s, %s, %v}", maxRevalDurationDays, *j.DeliveryService, *j.AssetURL, *j.StartTime)
+		}
+	}
+
+	// create DS xml_id -> cdn_id lookup map
+	dses, _, err := TOSession.GetDeliveryServices(client.NewRequestOptions())
+	if err != nil {
+		t.Fatalf("unexpectd error getting delivery services: %v", err)
+	}
+	dsToCDN := make(map[string]string, len(dses.Response))
+	for _, ds := range dses.Response {
+		dsToCDN[*ds.XMLID] = *ds.CDNName
+	}
+
+	cdn := "cdn2"
+	// get jobs by CDN ID
+	opts = client.NewRequestOptions()
+	opts.QueryParameters.Set("cdn", cdn)
+	cdnJobs, _, err := TOSession.GetInvalidationJobs(opts)
+	if err != nil {
+		t.Errorf("unexpected error getting jobs by cdn: %v", err)
+	} else if len(cdnJobs.Response) < 1 {
+		t.Errorf("GET /jobs by cdn - expected at least 1 job")
+	}
+	for _, j := range cdnJobs.Response {
+		if dsToCDN[*j.DeliveryService] != cdn {
+			t.Errorf("GET /jobs by cdn returned job that does not belong to CDN %s: {%s, %s, %v}", cdn, *j.DeliveryService, *j.AssetURL, *j.StartTime)

Review comment:
       These lines will segfault if the API returns a malformed response that has missing or null fields




-- 
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-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org