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 2020/02/10 21:16:13 UTC

[GitHub] [trafficcontrol] shamrickus opened a new pull request #4390: Remove methods marked as deprecated in 3.x

shamrickus opened a new pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390
 
 
   <!--
   ************ STOP!! ************
   If this Pull Request is intended to fix a security vulnerability, DO NOT submit it! Instead, contact
   the Apache Software Foundation Security Team at security@trafficcontrol.apache.org and follow the
   guidelines at https://www.apache.org/security/ regarding vulnerability disclosure.
   -->
   ## What does this PR (Pull Request) do?
   <!-- Explain the changes you made here. If this fixes an Issue, identify it by
   replacing the text in the checkbox item with the Issue number e.g.
   
   - [x] This PR fixes #9001 OR is not related to any Issue
   
   ^ This will automatically close Issue number 9001 when the Pull Request is
   merged (The '#' is important).
   
   Be sure you check the box properly, see the "The following criteria are ALL
   met by this PR" section for details.
   -->
   
   - [x] This PR fixes #3344
   
   ## Which Traffic Control components are affected by this PR?
   <!-- Please delete all components from this list that are NOT affected by this
   Pull Request. Also, feel free to add the name of a tool or script that is
   affected but not on the list.
   
   Additionally, if this Pull Request does NOT affect documentation, please
   explain why documentation is not required. -->
   
   - CDN in a Box
   - Grove
   - Traffic Ops
   - Traffic Stats
   
   ## What is the best way to verify this PR?
   <!-- Please include here ALL the steps necessary to test your Pull Request. If
   it includes tests (and most should), outline here the steps needed to run the
   tests. If not, lay out the manual testing procedure and please explain why
   tests are unnecessary for this Pull Request. -->
   Pull code and ensure build successful for affected modules as well as tests.
   
   <!-- If this PR fixes a bug, please list here all of the affected versions - to
   the best of your knowledge. It's also pretty helpful to include a commit hash
   of where 'master' is at the time this PR is opened (if it affects master),
   because what 'master' means will change over time. For example, if this PR
   fixes a bug that's present in master (at commit hash '2697ebac'), in v3.0.0,
   and in the current 3.0.1 Release candidate (e.g. RC1), then this list would
   look like:
   
   - master (2697ebac)
   - 3.0.0
   - 3.0.1 (RC1)
   
   If you don't know what other versions might have this bug, AND don't know how
   to find the commit hash of 'master', then feel free to leave this section
   blank (or, preferably, delete it entirely).
    -->
   
   
   ## The following criteria are ALL met by this PR
   <!-- Check the boxes to signify that the associated statement is true. To
   "check a box", replace the space inside of the square brackets with an 'x'.
   e.g.
   
   - [ x] <- Wrong
   - [x ] <- Wrong
   - [] <- Wrong
   - [*] <- Wrong
   - [x] <- Correct!
   
   -->
   
   - [x] This PR includes tests OR I have explained why tests are unnecessary
   - [x] This PR includes documentation OR I have explained why documentation is unnecessary
   - [x] This PR includes an update to CHANGELOG.md OR such an update is not necessary
   - [x] This PR includes any and all required license headers
   - [x] This PR ensures that database migration sequence is correct OR this PR does not include a database migration
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
   
   
   ## Additional Information
   <!-- If you would like to include any additional information on the PR for
   potential reviewers please put it here.
   
   Some examples of this would be:
   
   - Before and after screenshots/gifs of the Traffic Portal if it is affected
   - Links to other dependent Pull Requests
   - References to relevant context (e.g. new/updates to dependent libraries,
   mailing list records, blueprints)
   
   Feel free to leave this section blank (or, preferably, delete it entirely).
   -->
   
   <!--
   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.
   -->
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377323783
 
 

 ##########
 File path: grove/grovetccfg/grovetccfg.go
 ##########
 @@ -361,7 +361,7 @@ func createGroveCfg(toc *to.Session, server tc.Server) (bool, config.Config, err
 		}
 	}
 
-	serverParameters, err := toc.Parameters(server.Profile)
+	serverParameters, _, err := toc.GetParameterByName(server.Profile)
 
 Review comment:
   This should be using `toc.GetParametersByProfileName`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377324700
 
 

 ##########
 File path: infrastructure/cdn-in-a-box/enroller/enroller.go
 ##########
 @@ -73,14 +73,14 @@ func (s session) getParameter(m tc.Parameter) (tc.Parameter, error) {
 }
 
 func (s session) getDeliveryServiceIDByXMLID(n string) (int, error) {
-	dses, _, err := s.GetDeliveryServiceByXMLID(url.QueryEscape(n))
+	dses, _, err := s.GetDeliveryServiceByXMLIDNullable(url.QueryEscape(n))
 	if err != nil {
 		return -1, err
 	}
 	if len(dses) == 0 {
 		return -1, errors.New("no deliveryservice with name " + n)
 	}
-	return dses[0].ID, err
+	return *dses[0].ID, err
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rawlinp commented on issue #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
rawlinp commented on issue #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#issuecomment-584897331
 
 
   (previous comment is for the asf-ci to run, not sure why it's not asking if an admin can verify this patch...)

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 merged pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 merged pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327288
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -70,23 +70,23 @@ func CreateTestSteeringTargets(t *testing.T) {
 			st.TypeID = util.IntPtr(respTypes[0].ID)
 		}
 		{
-			respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+			respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 			if err != nil {
 				t.Errorf("creating steering target: getting ds: %v", err)
 			} else if len(respDS) < 1 {
 				t.Error("creating steering target: getting ds: not found")
 			}
-			dsID := uint64(respDS[0].ID)
+			dsID := uint64(*respDS[0].ID)
 			st.DeliveryServiceID = &dsID
 		}
 		{
-			respTarget, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.Target))
+			respTarget, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.Target))
 			if err != nil {
 				t.Errorf("creating steering target: getting target ds: %v", err)
 			} else if len(respTarget) < 1 {
 				t.Error("creating steering target: getting target ds: not found")
 			}
-			targetID := uint64(respTarget[0].ID)
+			targetID := uint64(*respTarget[0].ID)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377326717
 
 

 ##########
 File path: traffic_ops/testing/api/v1/jobs_test.go
 ##########
 @@ -93,36 +92,26 @@ func GetTestJobs(t *testing.T) {
 		t.Fatalf("cannot GET DeliveryServices: %v - %v", err, toDSes)
 	}
 
-	dsIDNames := map[int64]string{}
-	for _, ds := range toDSes {
-		dsIDNames[int64(ds.ID)] = ds.XMLID
-	}
-
-	for _, testJob := range testData.Jobs {
+	for _, testJob := range testData.InvalidationJobs {
 		found := false
 		for _, toJob := range toJobs {
-			if toJob.DeliveryService != dsIDNames[testJob.Request.DeliveryServiceID] {
+			if *toJob.DeliveryService != *testJob.DeliveryService {
 				continue
 			}
-			if !strings.HasSuffix(toJob.AssetURL, testJob.Request.Regex) {
-				continue
-			}
-			toJobTime, err := time.Parse(tc.TimeLayout, toJob.StartTime)
-			if err != nil {
-				t.Errorf("job ds %v regex %v start time expected format '%+v' actual '%+v' error '%+v'", testJob.Request.DeliveryServiceID, testJob.Request.Regex, tc.TimeLayout, toJob.StartTime, err)
+			if !strings.HasSuffix(*toJob.AssetURL, *testJob.Regex) {
 				continue
 			}
-			toJobTime = toJobTime.Round(time.Minute)
-			testJobTime := testJob.Request.StartTime.Round(time.Minute)
+			toJobTime := toJob.StartTime.Round(time.Minute)
+			testJobTime := testJob.StartTime.Round(time.Minute)
 			if !toJobTime.Equal(testJobTime) {
-				t.Errorf("test job ds %v regex %v start time expected '%+v' actual '%+v'", testJob.Request.DeliveryServiceID, testJob.Request.Regex, testJobTime, toJobTime)
+				t.Errorf("test job ds %v regex %v start time expected '%+v' actual '%+v'", *testJob.DeliveryService, *testJob.Regex, testJobTime, toJobTime)
 				continue
 			}
 			found = true
 			break
 		}
 		if !found {
-			t.Errorf("test job ds %v regex %v expected: exists, actual: not found", testJob.Request.DeliveryServiceID, testJob.Request.Regex)
+			t.Errorf("test job ds %v regex %v expected: exists, actual: not found", *testJob.DeliveryService, *testJob.Regex)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327858
 
 

 ##########
 File path: traffic_ops/testing/api/v1/tenants_test.go
 ##########
 @@ -263,24 +263,24 @@ func UpdateTestTenantsActive(t *testing.T) {
 	// 2. tenant3user has tenant3.
 	// 3. tenant2 is not a child of tenant3 (tenant3 is a child of tenant2)
 	// 4. Therefore, tenant3user should not have access to ds2
-	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLID("ds2")
+	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds2")
 	for _, ds := range dses {
-		t.Errorf("tenant3user got delivery service %+v with tenant2, expected: no ds", ds.XMLID)
+		t.Errorf("tenant3user got delivery service %+v with tenant2, expected: no ds", *ds.XMLID)
 	}
 
 	// 1. ds1 has tenant1.
 	// 2. tenant4user has tenant4.
 	// 3. tenant1 is not a child of tenant4 (tenant4 is unrelated to tenant1)
 	// 4. Therefore, tenant4user should not have access to ds1
-	dses, _, _ = tenant4Session.GetDeliveryServiceByXMLID("ds1")
+	dses, _, _ = tenant4Session.GetDeliveryServiceByXMLIDNullable("ds1")
 	for _, ds := range dses {
-		t.Errorf("tenant4user got delivery service %+v with tenant1, expected: no ds", ds.XMLID)
+		t.Errorf("tenant4user got delivery service %+v with tenant1, expected: no ds", *ds.XMLID)
 	}
 
 	setTenantActive(t, "tenant3", false)
-	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLID("ds3")
+	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds3")
 	for _, ds := range dses {
-		t.Errorf("tenant3user was inactive, but got delivery service %+v with tenant3, expected: no ds", ds.XMLID)
+		t.Errorf("tenant3user was inactive, but got delivery service %+v with tenant3, expected: no ds", *ds.XMLID)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327215
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -70,23 +70,23 @@ func CreateTestSteeringTargets(t *testing.T) {
 			st.TypeID = util.IntPtr(respTypes[0].ID)
 		}
 		{
-			respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+			respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 			if err != nil {
 				t.Errorf("creating steering target: getting ds: %v", err)
 			} else if len(respDS) < 1 {
 				t.Error("creating steering target: getting ds: not found")
 			}
-			dsID := uint64(respDS[0].ID)
+			dsID := uint64(*respDS[0].ID)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377325753
 
 

 ##########
 File path: traffic_ops/client/staticdnsentry.go
 ##########
 @@ -31,25 +31,25 @@ const (
 
 func staticDNSEntryIDs(to *Session, sdns *tc.StaticDNSEntry) error {
 	if sdns.CacheGroupID == 0 && sdns.CacheGroupName != "" {
-		p, _, err := to.GetCacheGroupByName(sdns.CacheGroupName)
+		p, _, err := to.GetCacheGroupNullableByName(sdns.CacheGroupName)
 		if err != nil {
 			return err
 		}
 		if len(p) == 0 {
 			return errors.New("no CacheGroup named " + sdns.CacheGroupName)
 		}
-		sdns.CacheGroupID = p[0].ID
+		sdns.CacheGroupID = *p[0].ID
 	}
 
 	if sdns.DeliveryServiceID == 0 && sdns.DeliveryService != "" {
-		dses, _, err := to.GetDeliveryServiceByXMLID(sdns.DeliveryService)
+		dses, _, err := to.GetDeliveryServiceByXMLIDNullable(sdns.DeliveryService)
 		if err != nil {
 			return err
 		}
 		if len(dses) == 0 {
 			return errors.New("no deliveryservice with name " + sdns.DeliveryService)
 		}
-		sdns.DeliveryServiceID = dses[0].ID
+		sdns.DeliveryServiceID = *dses[0].ID
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377326413
 
 

 ##########
 File path: traffic_ops/testing/api/v1/jobs_test.go
 ##########
 @@ -93,36 +92,26 @@ func GetTestJobs(t *testing.T) {
 		t.Fatalf("cannot GET DeliveryServices: %v - %v", err, toDSes)
 	}
 
-	dsIDNames := map[int64]string{}
-	for _, ds := range toDSes {
-		dsIDNames[int64(ds.ID)] = ds.XMLID
-	}
-
-	for _, testJob := range testData.Jobs {
+	for _, testJob := range testData.InvalidationJobs {
 		found := false
 		for _, toJob := range toJobs {
-			if toJob.DeliveryService != dsIDNames[testJob.Request.DeliveryServiceID] {
+			if *toJob.DeliveryService != *testJob.DeliveryService {
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377326484
 
 

 ##########
 File path: traffic_ops/testing/api/v1/jobs_test.go
 ##########
 @@ -93,36 +92,26 @@ func GetTestJobs(t *testing.T) {
 		t.Fatalf("cannot GET DeliveryServices: %v - %v", err, toDSes)
 	}
 
-	dsIDNames := map[int64]string{}
-	for _, ds := range toDSes {
-		dsIDNames[int64(ds.ID)] = ds.XMLID
-	}
-
-	for _, testJob := range testData.Jobs {
+	for _, testJob := range testData.InvalidationJobs {
 		found := false
 		for _, toJob := range toJobs {
-			if toJob.DeliveryService != dsIDNames[testJob.Request.DeliveryServiceID] {
+			if *toJob.DeliveryService != *testJob.DeliveryService {
 				continue
 			}
-			if !strings.HasSuffix(toJob.AssetURL, testJob.Request.Regex) {
-				continue
-			}
-			toJobTime, err := time.Parse(tc.TimeLayout, toJob.StartTime)
-			if err != nil {
-				t.Errorf("job ds %v regex %v start time expected format '%+v' actual '%+v' error '%+v'", testJob.Request.DeliveryServiceID, testJob.Request.Regex, tc.TimeLayout, toJob.StartTime, err)
+			if !strings.HasSuffix(*toJob.AssetURL, *testJob.Regex) {
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327093
 
 

 ##########
 File path: traffic_ops/testing/api/v1/servers_to_deliveryservice_assignment_test.go
 ##########
 @@ -89,15 +89,15 @@ func AssignIncorrectTestDeliveryService(t *testing.T) {
 	}
 	server = &rs[0]
 
-	rd, _, err := TOSession.GetDeliveryServiceByXMLID(testData.DeliveryServices[0].XMLID)
+	rd, _, err := TOSession.GetDeliveryServiceByXMLIDNullable(testData.DeliveryServices[0].XMLID)
 	if err != nil {
 		t.Fatalf("Failed to fetch DS information: %v", err)
 	} else if len(rd) == 0 {
 		t.Fatalf("Failed to fetch DS information: No results returned!")
 	}
 	firstDS := rd[0]
 
-	alerts, _, err := TOSession.AssignDeliveryServiceIDsToServerID(server.ID, []int{firstDS.ID}, false)
+	alerts, _, err := TOSession.AssignDeliveryServiceIDsToServerID(server.ID, []int{*firstDS.ID}, false)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377928575
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -51,80 +51,87 @@ func SetupSteeringTargets(t *testing.T) {
 func CreateTestSteeringTargets(t *testing.T) {
 	for _, st := range testData.SteeringTargets {
 		if st.Type == nil {
-			t.Error("creating steering target: test data missing type")
+			t.Fatal("creating steering target: test data missing type")
 		}
 		if st.DeliveryService == nil {
-			t.Error("creating steering target: test data missing ds")
+			t.Fatal("creating steering target: test data missing ds")
 		}
 		if st.Target == nil {
-			t.Error("creating steering target: test data missing target")
+			t.Fatal("creating steering target: test data missing target")
 		}
 
 		{
 			respTypes, _, err := SteeringUserSession.GetTypeByName(*st.Type)
 			if err != nil {
-				t.Errorf("creating steering target: getting type: %v", err)
+				t.Fatalf("creating steering target: getting type: %v", err)
 			} else if len(respTypes) < 1 {
-				t.Error("creating steering target: getting type: not found")
+				t.Fatal("creating steering target: getting type: not found")
 			}
 			st.TypeID = util.IntPtr(respTypes[0].ID)
 		}
 		{
-			respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+			respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 			if err != nil {
-				t.Errorf("creating steering target: getting ds: %v", err)
+				t.Fatalf("creating steering target: getting ds: %v", err)
 			} else if len(respDS) < 1 {
-				t.Error("creating steering target: getting ds: not found")
+				t.Fatal("creating steering target: getting ds: not found")
+			} else if respDS[0].ID == nil {
+				t.Fatal("creating steering target: getting ds: nil ID returned")
 			}
-			dsID := uint64(respDS[0].ID)
+			dsID := uint64(*respDS[0].ID)
 			st.DeliveryServiceID = &dsID
 		}
 		{
-			respTarget, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.Target))
+			respTarget, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.Target))
 			if err != nil {
-				t.Errorf("creating steering target: getting target ds: %v", err)
+				t.Fatalf("creating steering target: getting target ds: %v", err)
 			} else if len(respTarget) < 1 {
-				t.Error("creating steering target: getting target ds: not found")
+				t.Fatal("creating steering target: getting target ds: not found")
+			} else if respTarget[0].ID == nill {
 
 Review comment:
   > `nill`

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327983
 
 

 ##########
 File path: traffic_ops/testing/api/v1/tenants_test.go
 ##########
 @@ -263,24 +263,24 @@ func UpdateTestTenantsActive(t *testing.T) {
 	// 2. tenant3user has tenant3.
 	// 3. tenant2 is not a child of tenant3 (tenant3 is a child of tenant2)
 	// 4. Therefore, tenant3user should not have access to ds2
-	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLID("ds2")
+	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds2")
 	for _, ds := range dses {
-		t.Errorf("tenant3user got delivery service %+v with tenant2, expected: no ds", ds.XMLID)
+		t.Errorf("tenant3user got delivery service %+v with tenant2, expected: no ds", *ds.XMLID)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327337
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -110,14 +110,14 @@ func UpdateTestSteeringTargets(t *testing.T) {
 		t.Error("updating steering target: test data missing target")
 	}
 
-	respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+	respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 	if err != nil {
 		t.Errorf("updating steering target: getting ds: %v", err)
 	}
 	if len(respDS) < 1 {
 		t.Error("updating steering target: getting ds: not found")
 	}
-	dsID := respDS[0].ID
+	dsID := *respDS[0].ID
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327383
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -194,13 +194,13 @@ func GetTestSteeringTargets(t *testing.T) {
 		t.Error("updating steering target: test data missing ds")
 	}
 
-	respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+	respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 	if err != nil {
 		t.Errorf("creating steering target: getting ds: %v", err)
 	} else if len(respDS) < 1 {
 		t.Error("steering target get: getting ds: not found")
 	}
-	dsID := respDS[0].ID
+	dsID := *respDS[0].ID
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377324606
 
 

 ##########
 File path: grove/grovetccfg/grovetccfg.go
 ##########
 @@ -748,7 +748,7 @@ func getParents(hostname string, servers map[string]tc.Server, cachegroups map[s
 
 	parents := []tc.Server{}
 	for _, server := range servers {
-		if server.Cachegroup == cachegroup.ParentName {
+		if server.Cachegroup == *cachegroup.ParentName {
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377769338
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -251,24 +260,28 @@ func DeleteTestSteeringTargets(t *testing.T) {
 			t.Error("deleting steering target: test data missing target")
 		}
 
-		respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+		respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 		if err != nil {
 			t.Errorf("deleting steering target: getting ds: %v", err)
 		} else if len(respDS) < 1 {
 			t.Error("deleting steering target: getting ds: not found")
+		} else if respDS[0].ID == nil {
+			t.Error("deleting steering target: getting ds: nil ID returned")
 		}
-		dsID := uint64(respDS[0].ID)
+		dsID := uint64(*respDS[0].ID)
 		st.DeliveryServiceID = &dsID
 
 		dsIDs = append(dsIDs, dsID)
 
-		respTarget, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.Target))
+		respTarget, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.Target))
 		if err != nil {
 			t.Errorf("deleting steering target: getting target ds: %v", err)
 		} else if len(respTarget) < 1 {
 			t.Error("deleting steering target: getting target ds: not found")
+		} else if respDS[0].ID == nil {
+			t.Error("deleting steering target: getting target ds: not found")
 
 Review comment:
   Same as above RE: fatal errors

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] rawlinp commented on issue #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
rawlinp commented on issue #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#issuecomment-584897133
 
 
   add to whitelist

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377325456
 
 

 ##########
 File path: traffic_ops/client/server.go
 ##########
 @@ -40,14 +40,14 @@ func (to *Session) CreateServer(server tc.Server) (tc.Alerts, ReqInf, error) {
 	reqInf := ReqInf{CacheHitStatus: CacheHitStatusMiss, RemoteAddr: remoteAddr}
 
 	if server.CachegroupID == 0 && server.Cachegroup != "" {
-		cg, _, err := to.GetCacheGroupByName(server.Cachegroup)
+		cg, _, err := to.GetCacheGroupNullableByName(server.Cachegroup)
 		if err != nil {
 			return tc.Alerts{}, ReqInf{}, errors.New("no cachegroup named " + server.Cachegroup + ":" + err.Error())
 		}
 		if len(cg) == 0 {
 			return tc.Alerts{}, ReqInf{}, errors.New("no cachegroup named " + server.Cachegroup)
 		}
-		server.CachegroupID = cg[0].ID
+		server.CachegroupID = *cg[0].ID
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377324773
 
 

 ##########
 File path: infrastructure/cdn-in-a-box/enroller/enroller.go
 ##########
 @@ -227,14 +227,14 @@ func enrollDeliveryServiceServer(toSession *session, r io.Reader) error {
 		return err
 	}
 
-	dses, _, err := toSession.GetDeliveryServiceByXMLID(dss.XmlId)
+	dses, _, err := toSession.GetDeliveryServiceByXMLIDNullable(dss.XmlId)
 	if err != nil {
 		return err
 	}
 	if len(dses) == 0 {
 		return errors.New("no deliveryservice with name " + dss.XmlId)
 	}
-	dsID := dses[0].ID
+	dsID := *dses[0].ID
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377771217
 
 

 ##########
 File path: traffic_ops/testing/api/v1/tenants_test.go
 ##########
 @@ -263,24 +267,30 @@ func UpdateTestTenantsActive(t *testing.T) {
 	// 2. tenant3user has tenant3.
 	// 3. tenant2 is not a child of tenant3 (tenant3 is a child of tenant2)
 	// 4. Therefore, tenant3user should not have access to ds2
-	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLID("ds2")
+	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds2")
 	for _, ds := range dses {
-		t.Errorf("tenant3user got delivery service %+v with tenant2, expected: no ds", ds.XMLID)
+		if ds.XMLID != nil {
+			t.Errorf("tenant3user got delivery service %+v with tenant2, expected: no ds", *ds.XMLID)
+		}
 	}
 
 	// 1. ds1 has tenant1.
 	// 2. tenant4user has tenant4.
 	// 3. tenant1 is not a child of tenant4 (tenant4 is unrelated to tenant1)
 	// 4. Therefore, tenant4user should not have access to ds1
-	dses, _, _ = tenant4Session.GetDeliveryServiceByXMLID("ds1")
+	dses, _, _ = tenant4Session.GetDeliveryServiceByXMLIDNullable("ds1")
 	for _, ds := range dses {
-		t.Errorf("tenant4user got delivery service %+v with tenant1, expected: no ds", ds.XMLID)
+		if ds.XMLID != nil {
+			t.Errorf("tenant4user got delivery service %+v with tenant1, expected: no ds", *ds.XMLID)
 
 Review comment:
   Same as above RE: this is still an error

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327536
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -251,24 +251,24 @@ func DeleteTestSteeringTargets(t *testing.T) {
 			t.Error("deleting steering target: test data missing target")
 		}
 
-		respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+		respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 		if err != nil {
 			t.Errorf("deleting steering target: getting ds: %v", err)
 		} else if len(respDS) < 1 {
 			t.Error("deleting steering target: getting ds: not found")
 		}
-		dsID := uint64(respDS[0].ID)
+		dsID := uint64(*respDS[0].ID)
 		st.DeliveryServiceID = &dsID
 
 		dsIDs = append(dsIDs, dsID)
 
-		respTarget, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.Target))
+		respTarget, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.Target))
 		if err != nil {
 			t.Errorf("deleting steering target: getting target ds: %v", err)
 		} else if len(respTarget) < 1 {
 			t.Error("deleting steering target: getting target ds: not found")
 		}
-		targetID := uint64(respTarget[0].ID)
+		targetID := uint64(*respTarget[0].ID)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377326930
 
 

 ##########
 File path: traffic_ops/testing/api/v1/servers_to_deliveryservice_assignment_test.go
 ##########
 @@ -36,15 +36,15 @@ func AssignTestDeliveryService(t *testing.T) {
 	}
 	firstServer := rs[0]
 
-	rd, _, err := TOSession.GetDeliveryServiceByXMLID(testData.DeliveryServices[0].XMLID)
+	rd, _, err := TOSession.GetDeliveryServiceByXMLIDNullable(testData.DeliveryServices[0].XMLID)
 	if err != nil {
 		t.Fatalf("Failed to fetch DS information: %v", err)
 	} else if len(rd) == 0 {
 		t.Fatalf("Failed to fetch DS information: No results returned!")
 	}
 	firstDS := rd[0]
 
-	alerts, _, err := TOSession.AssignDeliveryServiceIDsToServerID(firstServer.ID, []int{firstDS.ID}, true)
+	alerts, _, err := TOSession.AssignDeliveryServiceIDsToServerID(firstServer.ID, []int{*firstDS.ID}, true)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327025
 
 

 ##########
 File path: traffic_ops/testing/api/v1/servers_to_deliveryservice_assignment_test.go
 ##########
 @@ -58,7 +58,7 @@ func AssignTestDeliveryService(t *testing.T) {
 
 	var found bool
 	for _, ds := range response {
-		if ds.ID != nil && *ds.ID == firstDS.ID {
+		if ds.ID != nil && *ds.ID == *firstDS.ID {
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327143
 
 

 ##########
 File path: traffic_ops/testing/api/v1/servers_to_deliveryservice_assignment_test.go
 ##########
 @@ -111,7 +111,7 @@ func AssignIncorrectTestDeliveryService(t *testing.T) {
 	var found bool
 	for _, ds := range response {
 
-		if ds.ID != nil && *ds.ID == firstDS.ID {
+		if ds.ID != nil && *ds.ID == *firstDS.ID {
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377768672
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -110,14 +114,17 @@ func UpdateTestSteeringTargets(t *testing.T) {
 		t.Error("updating steering target: test data missing target")
 	}
 
-	respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+	respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 	if err != nil {
 		t.Errorf("updating steering target: getting ds: %v", err)
 	}
 	if len(respDS) < 1 {
 		t.Error("updating steering target: getting ds: not found")
 	}
-	dsID := respDS[0].ID
+	if respDS[0].ID == nil {
+		t.Error("updating steering target: getting ds: nil id returned")
 
 Review comment:
   Same as above RE: fatal errors

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377770813
 
 

 ##########
 File path: traffic_ops/testing/api/v1/tenants_test.go
 ##########
 @@ -232,27 +232,31 @@ func UpdateTestTenantsActive(t *testing.T) {
 	}
 
 	// tenant3user with tenant3 has no access to ds3 with tenant3 when parent tenant2 is inactive
-	dses, _, err = tenant3Session.GetDeliveryServiceByXMLID("ds3")
+	dses, _, err = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds3")
 	for _, ds := range dses {
-		t.Errorf("tenant3user got delivery service %+v with tenant3 but tenant3 parent tenant2 is inactive, expected: no ds", ds.XMLID)
+		if ds.XMLID != nil {
+			t.Errorf("tenant3user got delivery service %+v with tenant3 but tenant3 parent tenant2 is inactive, expected: no ds", *ds.XMLID)
+		}
 	}
 
 	setTenantActive(t, "tenant1", true)
 	setTenantActive(t, "tenant2", true)
 	setTenantActive(t, "tenant3", false)
 
 	// tenant3user with tenant3 has no access to ds3 with tenant3 when tenant3 is inactive
-	dses, _, err = tenant3Session.GetDeliveryServiceByXMLID("ds3")
+	dses, _, err = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds3")
 	for _, ds := range dses {
-		t.Errorf("tenant3user got delivery service %+v with tenant3 but tenant3 is inactive, expected: no ds", ds.XMLID)
+		if ds.XMLID != nil {
+			t.Errorf("tenant3user got delivery service %+v with tenant3 but tenant3 is inactive, expected: no ds", *ds.XMLID)
 
 Review comment:
   This is still an error if `ds.XMLID` is `nil` - we expected no Delivery Services returned. IMO, a better way to check this is by looking at the length of `dses` and maybe also not just ignoring the error. You don't have to make that change, but this does need to be an error whether or not `ds.XMLID` is `nil`.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377325876
 
 

 ##########
 File path: traffic_ops/testing/api/v1/cachegroupsdeliveryservices_test.go
 ##########
 @@ -53,7 +53,7 @@ func CreateTestCachegroupsDeliveryServices(t *testing.T) {
 
 	clientCG := clientCGs[0]
 
-	cgID := clientCG.ID
+	cgID := *clientCG.ID
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377324028
 
 

 ##########
 File path: grove/grovetccfg/grovetccfg.go
 ##########
 @@ -464,27 +464,27 @@ func createRulesOldAPI(toc *to.Session, host string, certDir string, servers map
 		os.Exit(1)
 	}
 
-	deliveryservices, err := toc.DeliveryServicesByServer(hostServer.ID)
+	deliveryservices, _, err := toc.GetDeliveryServicesByServer(hostServer.ID)
 	if err != nil {
 		fmt.Println(time.Now().Format(time.RFC3339Nano) + " Error getting Traffic Ops Deliveryservices: " + err.Error())
 		os.Exit(1)
 	}
 
-	deliveryserviceRegexArr, err := toc.DeliveryServiceRegexes()
+	deliveryserviceRegexArr, _, err := toc.GetDeliveryServiceRegexes()
 	if err != nil {
 		fmt.Println(time.Now().Format(time.RFC3339Nano) + " Error getting Traffic Ops Deliveryservice Regexes: " + err.Error())
 		os.Exit(1)
 	}
 	deliveryserviceRegexes := makeDeliveryserviceRegexMap(deliveryserviceRegexArr)
 
-	cdnsArr, err := toc.CDNs()
+	cdnsArr, _, err := toc.GetCDNs()
 	if err != nil {
 		fmt.Println(time.Now().Format(time.RFC3339Nano) + " Error getting Traffic Ops CDNs: " + err.Error())
 		os.Exit(1)
 	}
 	cdns := makeCDNMap(cdnsArr)
 
-	serverParameters, err := toc.Parameters(hostServer.Profile)
+	serverParameters, _, err := toc.GetParameterByName(hostServer.Profile)
 
 Review comment:
   Same as above RE: getting by profile name, not parameter name

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377769185
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -251,24 +260,28 @@ func DeleteTestSteeringTargets(t *testing.T) {
 			t.Error("deleting steering target: test data missing target")
 		}
 
-		respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+		respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 		if err != nil {
 			t.Errorf("deleting steering target: getting ds: %v", err)
 		} else if len(respDS) < 1 {
 			t.Error("deleting steering target: getting ds: not found")
+		} else if respDS[0].ID == nil {
+			t.Error("deleting steering target: getting ds: nil ID returned")
 
 Review comment:
   Same as above RE: fatal errors

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327892
 
 

 ##########
 File path: traffic_ops/testing/api/v1/tenants_test.go
 ##########
 @@ -263,24 +263,24 @@ func UpdateTestTenantsActive(t *testing.T) {
 	// 2. tenant3user has tenant3.
 	// 3. tenant2 is not a child of tenant3 (tenant3 is a child of tenant2)
 	// 4. Therefore, tenant3user should not have access to ds2
-	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLID("ds2")
+	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds2")
 	for _, ds := range dses {
-		t.Errorf("tenant3user got delivery service %+v with tenant2, expected: no ds", ds.XMLID)
+		t.Errorf("tenant3user got delivery service %+v with tenant2, expected: no ds", *ds.XMLID)
 	}
 
 	// 1. ds1 has tenant1.
 	// 2. tenant4user has tenant4.
 	// 3. tenant1 is not a child of tenant4 (tenant4 is unrelated to tenant1)
 	// 4. Therefore, tenant4user should not have access to ds1
-	dses, _, _ = tenant4Session.GetDeliveryServiceByXMLID("ds1")
+	dses, _, _ = tenant4Session.GetDeliveryServiceByXMLIDNullable("ds1")
 	for _, ds := range dses {
-		t.Errorf("tenant4user got delivery service %+v with tenant1, expected: no ds", ds.XMLID)
+		t.Errorf("tenant4user got delivery service %+v with tenant1, expected: no ds", *ds.XMLID)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377324363
 
 

 ##########
 File path: grove/grovetccfg/grovetccfg.go
 ##########
 @@ -658,10 +658,10 @@ func makeServersHostnameMap(servers []tc.Server) map[string]tc.Server {
 	return m
 }
 
-func makeCachegroupsNameMap(cgs []tc.CacheGroup) map[string]tc.CacheGroup {
-	m := map[string]tc.CacheGroup{}
+func makeCachegroupsNameMap(cgs []tc.CacheGroupNullable) map[string]tc.CacheGroupNullable {
+	m := map[string]tc.CacheGroupNullable{}
 	for _, cg := range cgs {
-		m[cg.Name] = cg
+		m[*cg.Name] = cg
 
 Review comment:
   need to check for `nil` names now that we're using nullables, this could segfault

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377771334
 
 

 ##########
 File path: traffic_ops/testing/api/v1/tenants_test.go
 ##########
 @@ -263,24 +267,30 @@ func UpdateTestTenantsActive(t *testing.T) {
 	// 2. tenant3user has tenant3.
 	// 3. tenant2 is not a child of tenant3 (tenant3 is a child of tenant2)
 	// 4. Therefore, tenant3user should not have access to ds2
-	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLID("ds2")
+	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds2")
 	for _, ds := range dses {
-		t.Errorf("tenant3user got delivery service %+v with tenant2, expected: no ds", ds.XMLID)
+		if ds.XMLID != nil {
+			t.Errorf("tenant3user got delivery service %+v with tenant2, expected: no ds", *ds.XMLID)
+		}
 	}
 
 	// 1. ds1 has tenant1.
 	// 2. tenant4user has tenant4.
 	// 3. tenant1 is not a child of tenant4 (tenant4 is unrelated to tenant1)
 	// 4. Therefore, tenant4user should not have access to ds1
-	dses, _, _ = tenant4Session.GetDeliveryServiceByXMLID("ds1")
+	dses, _, _ = tenant4Session.GetDeliveryServiceByXMLIDNullable("ds1")
 	for _, ds := range dses {
-		t.Errorf("tenant4user got delivery service %+v with tenant1, expected: no ds", ds.XMLID)
+		if ds.XMLID != nil {
+			t.Errorf("tenant4user got delivery service %+v with tenant1, expected: no ds", *ds.XMLID)
+		}
 	}
 
 	setTenantActive(t, "tenant3", false)
-	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLID("ds3")
+	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds3")
 	for _, ds := range dses {
-		t.Errorf("tenant3user was inactive, but got delivery service %+v with tenant3, expected: no ds", ds.XMLID)
+		if ds.XMLID != nil {
+			t.Errorf("tenant3user was inactive, but got delivery service %+v with tenant3, expected: no ds", *ds.XMLID)
 
 Review comment:
   Same as above RE: this is still an error

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377326848
 
 

 ##########
 File path: traffic_ops/testing/api/v1/regexrevalidatedotconfig_test.go
 ##########
 @@ -32,9 +32,9 @@ func GetTestRegexRevalidateDotConfig(t *testing.T) {
 		t.Fatalf("Getting cdn '" + cdnName + "' config regex_revalidate.config: " + err.Error() + "\n")
 	}
 
-	for _, testJob := range testData.Jobs {
-		if !strings.Contains(cfg, testJob.Request.Regex) {
-			t.Errorf("regex_revalidate.config '''%+v''' expected: contains '%+v' actual: missing", cfg, testJob.Request.Regex)
+	for _, testJob := range testData.InvalidationJobs {
+		if !strings.Contains(cfg, *testJob.Regex) {
+			t.Errorf("regex_revalidate.config '''%+v''' expected: contains '%+v' actual: missing", cfg, *testJob.Regex)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327776
 
 

 ##########
 File path: traffic_stats/traffic_stats.go
 ##########
 @@ -459,7 +459,7 @@ func getToData(config StartupConfig, init bool, configChan chan RunningConfig) {
 
 	cacheStatPath := "/publish/CacheStats?hc=1&wildcard=1&stats="
 	dsStatPath := "/publish/DsStats?hc=1&wildcard=1&stats="
-	parameters, err := to.Parameters("TRAFFIC_STATS")
+	parameters, _, err := to.GetParameterByName("TRAFFIC_STATS")
 
 Review comment:
   Same as above RE: getting parameter by profile name instead of parameter name

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377325968
 
 

 ##########
 File path: traffic_ops/testing/api/v1/crconfig_test.go
 ##########
 @@ -61,14 +61,14 @@ func UpdateTestCRConfigSnapshot(t *testing.T) {
 	if serverID == 0 {
 		t.Errorf("GetServers expected EDGE server in cdn1, actual: %+v", servers)
 	}
-	res, _, err := TOSession.GetDeliveryServiceByXMLID("anymap-ds")
+	res, _, err := TOSession.GetDeliveryServiceByXMLIDNullable("anymap-ds")
 	if err != nil {
 		t.Errorf("GetDeliveryServiceByXMLID err expected nil, actual %+v", err)
 	}
 	if len(res) != 1 {
 		t.Error("GetDeliveryServiceByXMLID expected 1 DS, actual 0")
 	}
-	anymapDSID := res[0].ID
+	anymapDSID := *res[0].ID
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377327475
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -251,24 +251,24 @@ func DeleteTestSteeringTargets(t *testing.T) {
 			t.Error("deleting steering target: test data missing target")
 		}
 
-		respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+		respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 		if err != nil {
 			t.Errorf("deleting steering target: getting ds: %v", err)
 		} else if len(respDS) < 1 {
 			t.Error("deleting steering target: getting ds: not found")
 		}
-		dsID := uint64(respDS[0].ID)
+		dsID := uint64(*respDS[0].ID)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377328074
 
 

 ##########
 File path: traffic_ops/testing/api/v1/tenants_test.go
 ##########
 @@ -232,27 +232,27 @@ func UpdateTestTenantsActive(t *testing.T) {
 	}
 
 	// tenant3user with tenant3 has no access to ds3 with tenant3 when parent tenant2 is inactive
-	dses, _, err = tenant3Session.GetDeliveryServiceByXMLID("ds3")
+	dses, _, err = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds3")
 	for _, ds := range dses {
-		t.Errorf("tenant3user got delivery service %+v with tenant3 but tenant3 parent tenant2 is inactive, expected: no ds", ds.XMLID)
+		t.Errorf("tenant3user got delivery service %+v with tenant3 but tenant3 parent tenant2 is inactive, expected: no ds", *ds.XMLID)
 	}
 
 	setTenantActive(t, "tenant1", true)
 	setTenantActive(t, "tenant2", true)
 	setTenantActive(t, "tenant3", false)
 
 	// tenant3user with tenant3 has no access to ds3 with tenant3 when tenant3 is inactive
-	dses, _, err = tenant3Session.GetDeliveryServiceByXMLID("ds3")
+	dses, _, err = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds3")
 	for _, ds := range dses {
-		t.Errorf("tenant3user got delivery service %+v with tenant3 but tenant3 is inactive, expected: no ds", ds.XMLID)
+		t.Errorf("tenant3user got delivery service %+v with tenant3 but tenant3 is inactive, expected: no ds", *ds.XMLID)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377770952
 
 

 ##########
 File path: traffic_ops/testing/api/v1/tenants_test.go
 ##########
 @@ -263,24 +267,30 @@ func UpdateTestTenantsActive(t *testing.T) {
 	// 2. tenant3user has tenant3.
 	// 3. tenant2 is not a child of tenant3 (tenant3 is a child of tenant2)
 	// 4. Therefore, tenant3user should not have access to ds2
-	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLID("ds2")
+	dses, _, _ = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds2")
 	for _, ds := range dses {
-		t.Errorf("tenant3user got delivery service %+v with tenant2, expected: no ds", ds.XMLID)
+		if ds.XMLID != nil {
+			t.Errorf("tenant3user got delivery service %+v with tenant2, expected: no ds", *ds.XMLID)
 
 Review comment:
   Same as above RE this is still an error

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377768224
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -70,23 +70,27 @@ func CreateTestSteeringTargets(t *testing.T) {
 			st.TypeID = util.IntPtr(respTypes[0].ID)
 		}
 		{
-			respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+			respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 			if err != nil {
 				t.Errorf("creating steering target: getting ds: %v", err)
 			} else if len(respDS) < 1 {
 				t.Error("creating steering target: getting ds: not found")
+			} else if respDS[0].ID == nil {
+				t.Error("creating steering target: getting ds: nil ID returned")
 
 Review comment:
   This error needs to be fatal to actually prevent a segfault (the above two errors should also be fatal for similar reasons, but that's not your responsibility).

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377326677
 
 

 ##########
 File path: traffic_ops/testing/api/v1/jobs_test.go
 ##########
 @@ -93,36 +92,26 @@ func GetTestJobs(t *testing.T) {
 		t.Fatalf("cannot GET DeliveryServices: %v - %v", err, toDSes)
 	}
 
-	dsIDNames := map[int64]string{}
-	for _, ds := range toDSes {
-		dsIDNames[int64(ds.ID)] = ds.XMLID
-	}
-
-	for _, testJob := range testData.Jobs {
+	for _, testJob := range testData.InvalidationJobs {
 		found := false
 		for _, toJob := range toJobs {
-			if toJob.DeliveryService != dsIDNames[testJob.Request.DeliveryServiceID] {
+			if *toJob.DeliveryService != *testJob.DeliveryService {
 				continue
 			}
-			if !strings.HasSuffix(toJob.AssetURL, testJob.Request.Regex) {
-				continue
-			}
-			toJobTime, err := time.Parse(tc.TimeLayout, toJob.StartTime)
-			if err != nil {
-				t.Errorf("job ds %v regex %v start time expected format '%+v' actual '%+v' error '%+v'", testJob.Request.DeliveryServiceID, testJob.Request.Regex, tc.TimeLayout, toJob.StartTime, err)
+			if !strings.HasSuffix(*toJob.AssetURL, *testJob.Regex) {
 				continue
 			}
-			toJobTime = toJobTime.Round(time.Minute)
-			testJobTime := testJob.Request.StartTime.Round(time.Minute)
+			toJobTime := toJob.StartTime.Round(time.Minute)
+			testJobTime := testJob.StartTime.Round(time.Minute)
 			if !toJobTime.Equal(testJobTime) {
-				t.Errorf("test job ds %v regex %v start time expected '%+v' actual '%+v'", testJob.Request.DeliveryServiceID, testJob.Request.Regex, testJobTime, toJobTime)
+				t.Errorf("test job ds %v regex %v start time expected '%+v' actual '%+v'", *testJob.DeliveryService, *testJob.Regex, testJobTime, toJobTime)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377328144
 
 

 ##########
 File path: traffic_ops/testing/api/v1/tenants_test.go
 ##########
 @@ -232,27 +232,27 @@ func UpdateTestTenantsActive(t *testing.T) {
 	}
 
 	// tenant3user with tenant3 has no access to ds3 with tenant3 when parent tenant2 is inactive
-	dses, _, err = tenant3Session.GetDeliveryServiceByXMLID("ds3")
+	dses, _, err = tenant3Session.GetDeliveryServiceByXMLIDNullable("ds3")
 	for _, ds := range dses {
-		t.Errorf("tenant3user got delivery service %+v with tenant3 but tenant3 parent tenant2 is inactive, expected: no ds", ds.XMLID)
+		t.Errorf("tenant3user got delivery service %+v with tenant3 but tenant3 parent tenant2 is inactive, expected: no ds", *ds.XMLID)
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377326806
 
 

 ##########
 File path: traffic_ops/testing/api/v1/regexrevalidatedotconfig_test.go
 ##########
 @@ -32,9 +32,9 @@ func GetTestRegexRevalidateDotConfig(t *testing.T) {
 		t.Fatalf("Getting cdn '" + cdnName + "' config regex_revalidate.config: " + err.Error() + "\n")
 	}
 
-	for _, testJob := range testData.Jobs {
-		if !strings.Contains(cfg, testJob.Request.Regex) {
-			t.Errorf("regex_revalidate.config '''%+v''' expected: contains '%+v' actual: missing", cfg, testJob.Request.Regex)
+	for _, testJob := range testData.InvalidationJobs {
+		if !strings.Contains(cfg, *testJob.Regex) {
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377768382
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -70,23 +70,27 @@ func CreateTestSteeringTargets(t *testing.T) {
 			st.TypeID = util.IntPtr(respTypes[0].ID)
 		}
 		{
-			respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+			respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 			if err != nil {
 				t.Errorf("creating steering target: getting ds: %v", err)
 			} else if len(respDS) < 1 {
 				t.Error("creating steering target: getting ds: not found")
+			} else if respDS[0].ID == nil {
+				t.Error("creating steering target: getting ds: nil ID returned")
 			}
-			dsID := uint64(respDS[0].ID)
+			dsID := uint64(*respDS[0].ID)
 			st.DeliveryServiceID = &dsID
 		}
 		{
-			respTarget, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.Target))
+			respTarget, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.Target))
 			if err != nil {
 				t.Errorf("creating steering target: getting target ds: %v", err)
 			} else if len(respTarget) < 1 {
 				t.Error("creating steering target: getting target ds: not found")
+			} else if respTarget[0].ID == nill {
+				t.Error("creating steering target: getting target ds: nil ID returned")
 
 Review comment:
   Same as above RE: fatal errors

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377768909
 
 

 ##########
 File path: traffic_ops/testing/api/v1/steeringtargets_test.go
 ##########
 @@ -194,13 +201,15 @@ func GetTestSteeringTargets(t *testing.T) {
 		t.Error("updating steering target: test data missing ds")
 	}
 
-	respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLID(string(*st.DeliveryService))
+	respDS, _, err := SteeringUserSession.GetDeliveryServiceByXMLIDNullable(string(*st.DeliveryService))
 	if err != nil {
 		t.Errorf("creating steering target: getting ds: %v", err)
 	} else if len(respDS) < 1 {
 		t.Error("steering target get: getting ds: not found")
+	} else if respDS[0].ID == nil {
+		t.Error("steering target get: getting ds: nil id returned")
 
 Review comment:
   Same as above RE: Fatal errors

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4390: Remove methods marked as deprecated in 3.x
URL: https://github.com/apache/trafficcontrol/pull/4390#discussion_r377325681
 
 

 ##########
 File path: traffic_ops/client/staticdnsentry.go
 ##########
 @@ -31,25 +31,25 @@ const (
 
 func staticDNSEntryIDs(to *Session, sdns *tc.StaticDNSEntry) error {
 	if sdns.CacheGroupID == 0 && sdns.CacheGroupName != "" {
-		p, _, err := to.GetCacheGroupByName(sdns.CacheGroupName)
+		p, _, err := to.GetCacheGroupNullableByName(sdns.CacheGroupName)
 		if err != nil {
 			return err
 		}
 		if len(p) == 0 {
 			return errors.New("no CacheGroup named " + sdns.CacheGroupName)
 		}
-		sdns.CacheGroupID = p[0].ID
+		sdns.CacheGroupID = *p[0].ID
 
 Review comment:
   Same as above RE: segfault possibility

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services