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/06 16:33:00 UTC
[GitHub] [trafficcontrol] dmohan001c opened a new pull request #6164: Fixed create regions accepts mismatch values for divisionName
dmohan001c opened a new pull request #6164:
URL: https://github.com/apache/trafficcontrol/pull/6164
## What does this PR (Pull Request) do?
- [x] This PR fixes #5562 <!-- You can check for an issue here: https://github.com/apache/trafficcontrol/issues -->
This fixes the create regions accepts and returns mismatch values.
POST /regions
{
"name": "Manchester92",
"division": 1,
}
Response: {
"alerts": [
{
"text": "region was created.",
"level": "success"
}
],
"response": {
"divisionName": "Quebec",
"division": 1,
"id": 4,
"lastUpdated": "2021-09-06 16:21:26+00",
"name": "Manchester92"
}
}
## Which Traffic Control components are affected by this PR?
- CDN in a Box
- Traffic Control Client <!-- Please specify which; e.g. 'Python', 'Go', 'Java' -->
- Traffic Ops
- CI tests
## What is the best way to verify this PR?
Execute all the Integration tests and make sure the tests are passed.
## If this is a bug fix, what versions of Traffic Control are affected?
* Master
## 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 **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
--
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
[GitHub] [trafficcontrol] ocket8888 commented on pull request #6164: Fixed create regions accepts mismatch values for divisionName
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on pull request #6164:
URL: https://github.com/apache/trafficcontrol/pull/6164#issuecomment-920025910
I'm still seeing a mismatch between the `divisionName` and `division` properties in the POST response. Given the divisions:
```json
[
{
"id": 2,
"lastUpdated": "2021-09-15 13:23:50+00",
"name": "Quebec"
},
{
"id": 1,
"lastUpdated": "2021-09-15 13:23:50+00",
"name": "USA"
}
]
```
I made a POST request to `/regions` like so:
```http
POST /api/3.0/regions HTTP/1.1
User-Agent: python-requests/2.26.0
Accept-Encoding: gzip, deflate
Accept: */*
Connection: keep-alive
Cookie: mojolicious=...
Content-Length: 62
{"divisionName": "Quebec", "division": 1, "name": "testquest"}
```
Note that `divisionName` is the name of a Division with the ID `2`, and the name of the Division with ID `1` is `USA`. Now the response:
```http
HTTP/1.1 200 OK
Content-Encoding: gzip
Content-Type: application/json
Permissions-Policy: interest-cohort=()
Set-Cookie: mojolicious=...; Path=/; Expires=Wed, 15 Sep 2021 14:28:06 GMT; Max-Age=3600; HttpOnly
Vary: Accept-Encoding
X-Server-Name: traffic_ops_golang/
Date: Wed, 15 Sep 2021 13:28:06 GMT
Content-Length: 166
{
"alerts": [
{
"text": "region was created.",
"level": "success"
}
],
"response": {
"divisionName": "Quebec",
"division": 1,
"id": 3,
"lastUpdated": "2021-09-15 13:28:06+00",
"name": "testquest"
}
}
```
It still repeats what I put in, even though it's incorrect. A later request reveals the Region's *actual* `divisionName`:
```http
GET /api/3.0/regions?id=3 HTTP/1.1
User-Agent: python-requests/2.26.0
Accept-Encoding: gzip, deflate
Accept: */*
Connection: keep-alive
Cookie: mojolicious=...
```
```http
HTTP/1.1 200 OK
Content-Encoding: gzip
Content-Type: application/json
Permissions-Policy: interest-cohort=()
Set-Cookie: mojolicious=...; Path=/; Expires=Wed, 15 Sep 2021 14:35:47 GMT; Max-Age=3600; HttpOnly
Vary: Accept-Encoding
X-Server-Name: traffic_ops_golang/
Date: Wed, 15 Sep 2021 13:35:47 GMT
Content-Length: 128
{
"response": [
{
"divisionName": "USA",
"division": 1,
"id": 3,
"lastUpdated": "2021-09-15 13:28:06+00",
"name": "testquest"
}
]
}
```
--
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
[GitHub] [trafficcontrol] ocket8888 edited a comment on pull request #6164: Fixed create regions accepts mismatch values for divisionName
Posted by GitBox <gi...@apache.org>.
ocket8888 edited a comment on pull request #6164:
URL: https://github.com/apache/trafficcontrol/pull/6164#issuecomment-920025910
~~I'm still seeing a mismatch between the `divisionName` and `division` properties in the POST response.~~
No I'm not, I just have to make sure that changing branches succeeds before doing everything else.
--
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
[GitHub] [trafficcontrol] dmohan001c commented on a change in pull request #6164: Fixed create regions accepts mismatch values for divisionName
Posted by GitBox <gi...@apache.org>.
dmohan001c commented on a change in pull request #6164:
URL: https://github.com/apache/trafficcontrol/pull/6164#discussion_r708189060
##########
File path: traffic_ops/traffic_ops_golang/user/user.go
##########
@@ -36,7 +36,7 @@ import (
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
- "github.com/go-ozzo/ozzo-validation"
+ validation "github.com/go-ozzo/ozzo-validation"
Review comment:
removed.
--
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
[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6164: Fixed create regions accepts mismatch values for divisionName
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6164:
URL: https://github.com/apache/trafficcontrol/pull/6164#discussion_r707709113
##########
File path: traffic_ops/traffic_ops_golang/user/user.go
##########
@@ -36,7 +36,7 @@ import (
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/tenant"
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/util/ims"
- "github.com/go-ozzo/ozzo-validation"
+ validation "github.com/go-ozzo/ozzo-validation"
Review comment:
I don't think this PR needs to touch this file
##########
File path: traffic_ops/traffic_ops_golang/region/regions.go
##########
@@ -116,8 +117,38 @@ JOIN division d ON r.division = d.id ` + where + orderBy + pagination +
}
func (rg *TORegion) Update(h http.Header) (error, error, int) { return api.GenericUpdate(h, rg) }
-func (rg *TORegion) Create() (error, error, int) { return api.GenericCreate(rg) }
-func (rg *TORegion) Delete() (error, error, int) { return api.GenericDelete(rg) }
+
+func (rg *TORegion) Create() (error, error, int) {
+ resultRows, err := rg.APIInfo().Tx.NamedQuery(rg.InsertQuery(), rg)
+ if err != nil {
+ return api.ParseDBError(err)
+ }
+ defer resultRows.Close()
+
+ var divisionName string
+ var id int
+ var lastUpdated tc.TimeNoMod
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err = resultRows.Scan(&id, &lastUpdated, &divisionName); err != nil {
+ return nil, fmt.Errorf("could not scan after insert: %s\n)", err), http.StatusInternalServerError
Review comment:
Error strings should not contain newlines, also this should wrap `err` using `%w` instead of coercing it to a string with `%s`
--
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
[GitHub] [trafficcontrol] dmohan001c commented on a change in pull request #6164: Fixed create regions accepts mismatch values for divisionName
Posted by GitBox <gi...@apache.org>.
dmohan001c commented on a change in pull request #6164:
URL: https://github.com/apache/trafficcontrol/pull/6164#discussion_r708189364
##########
File path: traffic_ops/traffic_ops_golang/region/regions.go
##########
@@ -116,8 +117,38 @@ JOIN division d ON r.division = d.id ` + where + orderBy + pagination +
}
func (rg *TORegion) Update(h http.Header) (error, error, int) { return api.GenericUpdate(h, rg) }
-func (rg *TORegion) Create() (error, error, int) { return api.GenericCreate(rg) }
-func (rg *TORegion) Delete() (error, error, int) { return api.GenericDelete(rg) }
+
+func (rg *TORegion) Create() (error, error, int) {
+ resultRows, err := rg.APIInfo().Tx.NamedQuery(rg.InsertQuery(), rg)
+ if err != nil {
+ return api.ParseDBError(err)
+ }
+ defer resultRows.Close()
+
+ var divisionName string
+ var id int
+ var lastUpdated tc.TimeNoMod
+
+ rowsAffected := 0
+ for resultRows.Next() {
+ rowsAffected++
+ if err = resultRows.Scan(&id, &lastUpdated, &divisionName); err != nil {
+ return nil, fmt.Errorf("could not scan after insert: %s\n)", err), http.StatusInternalServerError
Review comment:
removed the new line and updated %w.
--
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
[GitHub] [trafficcontrol] ocket8888 merged pull request #6164: Fixed create regions accepts mismatch values for divisionName
Posted by GitBox <gi...@apache.org>.
ocket8888 merged pull request #6164:
URL: https://github.com/apache/trafficcontrol/pull/6164
--
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