[GitHub] [trafficcontrol] zrhoffman commented on a diff in pull request #6544: Layered profile `servers/` API {GET, POST, PUT & DELETE}

[GitBox <gi...@apache.org>]

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


##########
traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go:
##########
@@ -1979,3 +1979,111 @@ WHERE server.id = $2;`
 
 	return nil
 }
+
+// GetCommonServerPropertiesFromV4 converts CommonServerPropertiesV40 to CommonServerProperties struct.
+func GetCommonServerPropertiesFromV4(s tc.ServerV40, tx *sql.Tx) (tc.CommonServerProperties, error) {
+	var id int
+	var desc string
+	rows, err := tx.Query("SELECT id, description from profile WHERE name=$1", (s.ProfileNames)[0])

Review Comment:
   This line panics (in API versions 2.0 and 3.0) if `s.ProfileNames` is nil/empty. In theory, this should not happen if the migration ran successfully, but since the DB can sometimes get in an inconsistent state if an operator tries to restore it, `dbhelpers.GetCommonServerPropertiesFromV4()` should make sure ProfileNames has a nonzero length before trying to get the first element.
   
   I noticed this when testing the PR in the CDN in a Box for Developers (which seeds data straight from [`seed.psql`](https://github.com/apache/trafficcontrol/blob/e402b074a5/dev/traffic_ops/seed.psql), which has no `server_profile` table yet.



##########
traffic_ops/app/db/migrations/2022032914235900_add_server_profile_table.up.sql:
##########
@@ -0,0 +1,38 @@
+/*

Review Comment:
   `2022040623082100_server_config_update.down.sql` and `2022040623082100_server_config_update.up.sql` were added in #6730. so now the timestamp in the filename of the layered profile up/down migrations should be increased from `2022032914235900` to some value greater than `2022040623082100` so it is the last migration in the list when #6544 gets merged.



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