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/04/03 22:29:42 UTC
[GitHub] [trafficcontrol] rob05c opened a new pull request #4599: Add
Traffic Ops /server/update
rob05c opened a new pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599
Needed by ORT. The only existing endpoint do set reval without
overwriting existing values and creating race conditions is /update
outside the API.
Includes docs, tests, changelog.
- [x] This PR is not related to any Issue
## Which Traffic Control components are affected by this PR?
- Traffic Ops
## What is the best way to verify this PR?
Run tests. Request new endpoint, verify it works as expected, queue and reval flag are updated or not updated as requested.
## If this is a bug fix, what versions of Traffic Control are affected?
Not a bug fix.
## The following criteria are ALL met by this PR
- [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
----------------------------------------------------------------
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
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r405672397
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,81 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
+
+*************************************
+``servers/{{HostName-Or-ID}}/update``
+*************************************
+
+``POST``
+========
+:term:`Queue` or dequeue updates and revalidation updates for a specific server.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type: Object
+
+Request Structure
+-----------------
+.. table:: Request Path Parameters
+
+ +------------------+---------------------------------------------------------------------------------------------------------+
+ | Name | Description |
+ +==================+=========================================================================================================+
+ | HostName-OR-ID | The hostName or integral, unique identifier of the server on which updates are being queued or dequeued |
+ +------------------+---------------------------------------------------------------------------------------------------------+
+
+.. table:: Request Query Parameters
+
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | Name | Required | Description |
+ +===============+==========+======================================================================================+
+ | updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | reval_updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
+ +---------------+----------+--------------------------------------------------------------------------------------+
+
+.. code-block:: http
+ :caption: Request Example
+
+ GET /api/2.0/servers/my-edge/update?updated=true&reval_updated=false HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+
+Response Structure
+------------------
+A human-readable string explaining the value(s) updated.
Review comment:
You can just have the example since there's no `response` object. So basically you can get rid of this line.
----------------------------------------------------------------
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] rob05c commented on a change in pull request
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r404287607
##########
File path: traffic_ops/traffic_ops_golang/server/update.go
##########
@@ -0,0 +1,155 @@
+package server
+
+/*
+ * 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.
+ */
+
+import (
+ "database/sql"
+ "errors"
+ "net/http"
+ "strconv"
+ "strings"
+
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+)
+
+// UpdateHandler implements an http handler that updates a server's upd_pending and reval_pending values.
+func UpdateHandler(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"id-or-name"}, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ idOrName := inf.Params["id-or-name"]
+ id, err := strconv.Atoi(idOrName)
+ hostName := ""
+ if err == nil {
+ name, ok, err := dbhelpers.GetServerNameFromID(inf.Tx.Tx, id)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server name from id '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server ID '"+idOrName+"' not found"), nil)
+ return
+ }
+ hostName = name
+ } else {
+ hostName = idOrName
+ if _, ok, err := dbhelpers.GetServerIDFromName(hostName, inf.Tx.Tx); err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server id from name '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server name '"+idOrName+"' not found"), nil)
+ return
+ }
+ }
+
+ updated, hasUpdated := inf.Params["updated"]
+ revalUpdated, hasRevalUpdated := inf.Params["reval_updated"]
+ if !hasUpdated && !hasRevalUpdated {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("Must pass at least one query paramter of 'updated' or 'reval_updated'"), nil)
+ return
+ }
+ updated = strings.ToLower(updated)
+ revalUpdated = strings.ToLower(revalUpdated)
+
+ if hasUpdated && !strings.HasPrefix(updated, "t") && !strings.HasPrefix(updated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'updated' must be 'true' or 'false'"), nil)
+ return
+ }
+ if hasRevalUpdated && !strings.HasPrefix(revalUpdated, "t") && !strings.HasPrefix(revalUpdated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'reval_updated' must be 'true' or 'false'"), nil)
+ return
+ }
+
+ strToBool := func(s string) bool {
+ return !strings.HasPrefix(strings.ToLower(s), "f")
+ }
+
+ updatedPtr := (*bool)(nil)
+ if hasUpdated {
+ updatedBool := strToBool(updated)
+ updatedPtr = &updatedBool
+ }
+ revalUpdatedPtr := (*bool)(nil)
+ if hasRevalUpdated {
+ revalUpdatedBool := strToBool(revalUpdated)
+ revalUpdatedPtr = &revalUpdatedBool
+ }
Review comment:
As above, I'd prefer to accept what we can understand, for Robustness.
----------------------------------------------------------------
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] rob05c commented on a change in pull request
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r405589316
##########
File path: traffic_ops/traffic_ops_golang/server/update.go
##########
@@ -0,0 +1,155 @@
+package server
+
+/*
+ * 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.
+ */
+
+import (
+ "database/sql"
+ "errors"
+ "net/http"
+ "strconv"
+ "strings"
+
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+)
+
+// UpdateHandler implements an http handler that updates a server's upd_pending and reval_pending values.
+func UpdateHandler(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"id-or-name"}, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ idOrName := inf.Params["id-or-name"]
+ id, err := strconv.Atoi(idOrName)
+ hostName := ""
+ if err == nil {
+ name, ok, err := dbhelpers.GetServerNameFromID(inf.Tx.Tx, id)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server name from id '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server ID '"+idOrName+"' not found"), nil)
+ return
+ }
+ hostName = name
+ } else {
+ hostName = idOrName
+ if _, ok, err := dbhelpers.GetServerIDFromName(hostName, inf.Tx.Tx); err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server id from name '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server name '"+idOrName+"' not found"), nil)
+ return
+ }
+ }
+
+ updated, hasUpdated := inf.Params["updated"]
+ revalUpdated, hasRevalUpdated := inf.Params["reval_updated"]
+ if !hasUpdated && !hasRevalUpdated {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("Must pass at least one query paramter of 'updated' or 'reval_updated'"), nil)
+ return
+ }
+ updated = strings.ToLower(updated)
+ revalUpdated = strings.ToLower(revalUpdated)
+
+ if hasUpdated && !strings.HasPrefix(updated, "t") && !strings.HasPrefix(updated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'updated' must be 'true' or 'false'"), nil)
+ return
+ }
+ if hasRevalUpdated && !strings.HasPrefix(revalUpdated, "t") && !strings.HasPrefix(revalUpdated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'reval_updated' must be 'true' or 'false'"), nil)
+ return
+ }
+
+ strToBool := func(s string) bool {
+ return !strings.HasPrefix(strings.ToLower(s), "f")
+ }
+
+ updatedPtr := (*bool)(nil)
+ if hasUpdated {
+ updatedBool := strToBool(updated)
+ updatedPtr = &updatedBool
+ }
+ revalUpdatedPtr := (*bool)(nil)
+ if hasRevalUpdated {
+ revalUpdatedBool := strToBool(revalUpdated)
+ revalUpdatedPtr = &revalUpdatedBool
+ }
Review comment:
Done
----------------------------------------------------------------
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] rob05c commented on a change in pull request
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r405846104
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,81 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
+
+*************************************
+``servers/{{HostName-Or-ID}}/update``
+*************************************
+
+``POST``
+========
+:term:`Queue` or dequeue updates and revalidation updates for a specific server.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type: Object
Review comment:
Fixed
----------------------------------------------------------------
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] rob05c commented on a change in pull request
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r405603521
##########
File path: traffic_ops/traffic_ops_golang/server/update.go
##########
@@ -0,0 +1,155 @@
+package server
+
+/*
+ * 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.
+ */
+
+import (
+ "database/sql"
+ "errors"
+ "net/http"
+ "strconv"
+ "strings"
+
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+)
+
+// UpdateHandler implements an http handler that updates a server's upd_pending and reval_pending values.
+func UpdateHandler(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"id-or-name"}, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ idOrName := inf.Params["id-or-name"]
+ id, err := strconv.Atoi(idOrName)
+ hostName := ""
+ if err == nil {
+ name, ok, err := dbhelpers.GetServerNameFromID(inf.Tx.Tx, id)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server name from id '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server ID '"+idOrName+"' not found"), nil)
+ return
+ }
+ hostName = name
+ } else {
+ hostName = idOrName
+ if _, ok, err := dbhelpers.GetServerIDFromName(hostName, inf.Tx.Tx); err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server id from name '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server name '"+idOrName+"' not found"), nil)
+ return
+ }
+ }
+
+ updated, hasUpdated := inf.Params["updated"]
+ revalUpdated, hasRevalUpdated := inf.Params["reval_updated"]
+ if !hasUpdated && !hasRevalUpdated {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("Must pass at least one query paramter of 'updated' or 'reval_updated'"), nil)
+ return
+ }
+ updated = strings.ToLower(updated)
+ revalUpdated = strings.ToLower(revalUpdated)
+
+ if hasUpdated && !strings.HasPrefix(updated, "t") && !strings.HasPrefix(updated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'updated' must be 'true' or 'false'"), nil)
+ return
+ }
+ if hasRevalUpdated && !strings.HasPrefix(revalUpdated, "t") && !strings.HasPrefix(revalUpdated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'reval_updated' must be 'true' or 'false'"), nil)
+ return
+ }
+
+ strToBool := func(s string) bool {
+ return !strings.HasPrefix(strings.ToLower(s), "f")
+ }
+
+ updatedPtr := (*bool)(nil)
+ if hasUpdated {
+ updatedBool := strToBool(updated)
+ updatedPtr = &updatedBool
+ }
+ revalUpdatedPtr := (*bool)(nil)
+ if hasRevalUpdated {
+ revalUpdatedBool := strToBool(revalUpdated)
+ revalUpdatedPtr = &revalUpdatedBool
+ }
+
+ if err := setUpdateStatuses(inf.Tx.Tx, hostName, updatedPtr, revalUpdatedPtr); err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("setting updated statuses: "+err.Error()))
+ return
+ }
+
+ err = api.CreateChangeLogBuildMsg(
+ api.ApiChange,
+ api.Updated,
+ inf.User,
+ inf.Tx.Tx,
+ "server-update-status",
+ hostName,
+ map[string]interface{}{"host_name": hostName},
+ )
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("writing changelog: "+err.Error()))
+ return
+ }
+
+ respMsg := "successfully set server '" + hostName + "'"
+ if hasUpdated {
+ respMsg += " updated=" + strconv.FormatBool(strToBool(updated))
+ }
+ if hasRevalUpdated {
+ respMsg += " reval_updated=" + strconv.FormatBool(strToBool(revalUpdated))
+ }
+ api.WriteResp(w, r, respMsg)
Review comment:
Done
----------------------------------------------------------------
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] rob05c commented on a change in pull request
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r404287261
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,81 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
+
+*******************************
+``servers/{{HostName-Or-ID}}/update``
+*******************************
+
+``POST``
+========
+:term:`Queue` or dequeue updates and revalidation updates for a specific server.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type: Object
+
+Request Structure
+-----------------
+.. table:: Request Path Parameters
+
+ +------------------+---------------------------------------------------------------------------------------------------------+
+ | Name | Description |
+ +==================+=========================================================================================================+
+ | HostName-OR-ID | The hostName or integral, unique identifier of the server on which updates are being queued or dequeued |
+ +------------------+---------------------------------------------------------------------------------------------------------+
+
+.. table:: Request Query Parameters
+
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | Name | Required | Description |
+ +===============+==========+======================================================================================+
+ | updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | reval_updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
Review comment:
If you're ok with it, I'd prefer to document it as exactly `true` or `false`, even though the server supports more. I'm a big fan of the Robustness Principle ("Be liberal in what you accept, conservative in what you send"), I think it makes interfaces more robust and resilient, and easier to integrate with.
Documenting exactly those case-sensitive values will encourage clients to be "conservative in what they send," even though our server will try to understand and accept more for robustness.
----------------------------------------------------------------
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 #4599: Add Traffic
Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 merged pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599
----------------------------------------------------------------
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
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r405671611
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,81 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
+
+*************************************
+``servers/{{HostName-Or-ID}}/update``
+*************************************
+
+``POST``
+========
+:term:`Queue` or dequeue updates and revalidation updates for a specific server.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type: Object
+
+Request Structure
+-----------------
+.. table:: Request Path Parameters
+
+ +------------------+---------------------------------------------------------------------------------------------------------+
+ | Name | Description |
+ +==================+=========================================================================================================+
+ | HostName-OR-ID | The hostName or integral, unique identifier of the server on which updates are being queued or dequeued |
+ +------------------+---------------------------------------------------------------------------------------------------------+
+
+.. table:: Request Query Parameters
+
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | Name | Required | Description |
+ +===============+==========+======================================================================================+
+ | updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | reval_updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
+ +---------------+----------+--------------------------------------------------------------------------------------+
+
+.. code-block:: http
+ :caption: Request Example
+
+ GET /api/2.0/servers/my-edge/update?updated=true&reval_updated=false HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+
+Response Structure
+------------------
+A human-readable string explaining the value(s) updated.
+
+.. code-block:: http
+ :caption: Response Example
+
+ HTTP/1.1 200 OK
+ Access-Control-Allow-Credentials: true
+ Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept
+ Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+ Access-Control-Allow-Origin: *
+ Cache-Control: no-cache, no-store, max-age=0, must-revalidate
+ Content-Type: application/json
+ Date: Mon, 10 Dec 2018 18:20:04 GMT
+ X-Server-Name: traffic_ops_golang/
+ Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly
+ Vary: Accept-Encoding
+ Whole-Content-Sha512: 9Mmo9hIFZyF5gAvfdJD//VH9eNgiHVLinXt88H0GlJSHhwND8gMxaFyC+f9XZfiNAoGd1MKi1934ZJGmaIR6qQ==
+ Content-Length: 49
+ {
+ "response":"successfully set server 'my-edge' updated=true reval_updated=false"
+ }
Review comment:
This response example is no longer correct. Also, you need to have a blank line between the headers and the payload.
----------------------------------------------------------------
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] rob05c commented on a change in pull request
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r405846066
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,81 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
+
+*************************************
+``servers/{{HostName-Or-ID}}/update``
+*************************************
+
+``POST``
+========
+:term:`Queue` or dequeue updates and revalidation updates for a specific server.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type: Object
+
+Request Structure
+-----------------
+.. table:: Request Path Parameters
+
+ +------------------+---------------------------------------------------------------------------------------------------------+
+ | Name | Description |
+ +==================+=========================================================================================================+
+ | HostName-OR-ID | The hostName or integral, unique identifier of the server on which updates are being queued or dequeued |
+ +------------------+---------------------------------------------------------------------------------------------------------+
+
+.. table:: Request Query Parameters
+
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | Name | Required | Description |
+ +===============+==========+======================================================================================+
+ | updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | reval_updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
+ +---------------+----------+--------------------------------------------------------------------------------------+
+
+.. code-block:: http
+ :caption: Request Example
+
+ GET /api/2.0/servers/my-edge/update?updated=true&reval_updated=false HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+
+Response Structure
+------------------
+A human-readable string explaining the value(s) updated.
+
+.. code-block:: http
+ :caption: Response Example
+
+ HTTP/1.1 200 OK
+ Access-Control-Allow-Credentials: true
+ Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, Accept
+ Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+ Access-Control-Allow-Origin: *
+ Cache-Control: no-cache, no-store, max-age=0, must-revalidate
+ Content-Type: application/json
+ Date: Mon, 10 Dec 2018 18:20:04 GMT
+ X-Server-Name: traffic_ops_golang/
+ Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 18 Nov 2019 17:40:54 GMT; Max-Age=3600; HttpOnly
+ Vary: Accept-Encoding
+ Whole-Content-Sha512: 9Mmo9hIFZyF5gAvfdJD//VH9eNgiHVLinXt88H0GlJSHhwND8gMxaFyC+f9XZfiNAoGd1MKi1934ZJGmaIR6qQ==
+ Content-Length: 49
+ {
+ "response":"successfully set server 'my-edge' updated=true reval_updated=false"
+ }
Review comment:
Fixed
----------------------------------------------------------------
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
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r405671848
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,81 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
+
+*************************************
+``servers/{{HostName-Or-ID}}/update``
+*************************************
+
+``POST``
+========
+:term:`Queue` or dequeue updates and revalidation updates for a specific server.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type: Object
Review comment:
Response type is now `undefined`
----------------------------------------------------------------
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] rob05c commented on a change in pull request
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r405584957
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,81 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
+
+*******************************
+``servers/{{HostName-Or-ID}}/update``
+*******************************
Review comment:
Done
----------------------------------------------------------------
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
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r404410485
##########
File path: traffic_ops/traffic_ops_golang/server/update.go
##########
@@ -0,0 +1,155 @@
+package server
+
+/*
+ * 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.
+ */
+
+import (
+ "database/sql"
+ "errors"
+ "net/http"
+ "strconv"
+ "strings"
+
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+)
+
+// UpdateHandler implements an http handler that updates a server's upd_pending and reval_pending values.
+func UpdateHandler(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"id-or-name"}, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ idOrName := inf.Params["id-or-name"]
+ id, err := strconv.Atoi(idOrName)
+ hostName := ""
+ if err == nil {
+ name, ok, err := dbhelpers.GetServerNameFromID(inf.Tx.Tx, id)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server name from id '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server ID '"+idOrName+"' not found"), nil)
+ return
+ }
+ hostName = name
+ } else {
+ hostName = idOrName
+ if _, ok, err := dbhelpers.GetServerIDFromName(hostName, inf.Tx.Tx); err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server id from name '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server name '"+idOrName+"' not found"), nil)
+ return
+ }
+ }
+
+ updated, hasUpdated := inf.Params["updated"]
+ revalUpdated, hasRevalUpdated := inf.Params["reval_updated"]
+ if !hasUpdated && !hasRevalUpdated {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("Must pass at least one query paramter of 'updated' or 'reval_updated'"), nil)
+ return
+ }
+ updated = strings.ToLower(updated)
+ revalUpdated = strings.ToLower(revalUpdated)
+
+ if hasUpdated && !strings.HasPrefix(updated, "t") && !strings.HasPrefix(updated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'updated' must be 'true' or 'false'"), nil)
+ return
+ }
+ if hasRevalUpdated && !strings.HasPrefix(revalUpdated, "t") && !strings.HasPrefix(revalUpdated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'reval_updated' must be 'true' or 'false'"), nil)
+ return
+ }
+
+ strToBool := func(s string) bool {
+ return !strings.HasPrefix(strings.ToLower(s), "f")
+ }
+
+ updatedPtr := (*bool)(nil)
+ if hasUpdated {
+ updatedBool := strToBool(updated)
+ updatedPtr = &updatedBool
+ }
+ revalUpdatedPtr := (*bool)(nil)
+ if hasRevalUpdated {
+ revalUpdatedBool := strToBool(revalUpdated)
+ revalUpdatedPtr = &revalUpdatedBool
+ }
Review comment:
> "Hm, `0` and `no` feel weird to me."
I don't actually think those are worth implementing.
> "How about just case-insensitive `t`/`true`/`f`/`false`?"
That works for me.
----------------------------------------------------------------
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] rob05c commented on a change in pull request
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r406240321
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,87 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
Review comment:
Done
----------------------------------------------------------------
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
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r404263104
##########
File path: traffic_ops/traffic_ops_golang/server/update.go
##########
@@ -0,0 +1,155 @@
+package server
+
+/*
+ * 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.
+ */
+
+import (
+ "database/sql"
+ "errors"
+ "net/http"
+ "strconv"
+ "strings"
+
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+)
+
+// UpdateHandler implements an http handler that updates a server's upd_pending and reval_pending values.
+func UpdateHandler(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"id-or-name"}, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ idOrName := inf.Params["id-or-name"]
+ id, err := strconv.Atoi(idOrName)
+ hostName := ""
+ if err == nil {
+ name, ok, err := dbhelpers.GetServerNameFromID(inf.Tx.Tx, id)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server name from id '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server ID '"+idOrName+"' not found"), nil)
+ return
+ }
+ hostName = name
+ } else {
+ hostName = idOrName
+ if _, ok, err := dbhelpers.GetServerIDFromName(hostName, inf.Tx.Tx); err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server id from name '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server name '"+idOrName+"' not found"), nil)
+ return
+ }
+ }
+
+ updated, hasUpdated := inf.Params["updated"]
+ revalUpdated, hasRevalUpdated := inf.Params["reval_updated"]
+ if !hasUpdated && !hasRevalUpdated {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("Must pass at least one query paramter of 'updated' or 'reval_updated'"), nil)
+ return
+ }
+ updated = strings.ToLower(updated)
+ revalUpdated = strings.ToLower(revalUpdated)
+
+ if hasUpdated && !strings.HasPrefix(updated, "t") && !strings.HasPrefix(updated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'updated' must be 'true' or 'false'"), nil)
+ return
+ }
+ if hasRevalUpdated && !strings.HasPrefix(revalUpdated, "t") && !strings.HasPrefix(revalUpdated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'reval_updated' must be 'true' or 'false'"), nil)
+ return
+ }
+
+ strToBool := func(s string) bool {
+ return !strings.HasPrefix(strings.ToLower(s), "f")
+ }
+
+ updatedPtr := (*bool)(nil)
+ if hasUpdated {
+ updatedBool := strToBool(updated)
+ updatedPtr = &updatedBool
+ }
+ revalUpdatedPtr := (*bool)(nil)
+ if hasRevalUpdated {
+ revalUpdatedBool := strToBool(revalUpdated)
+ revalUpdatedPtr = &revalUpdatedBool
+ }
Review comment:
The only allowed values are "true" and "false" as indicated in your documentation and even the alerts you return, so there's no need to accept everything with certain prefixes. Just check for value equality with the allowed values.
----------------------------------------------------------------
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
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r404263883
##########
File path: traffic_ops/traffic_ops_golang/server/update.go
##########
@@ -0,0 +1,155 @@
+package server
+
+/*
+ * 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.
+ */
+
+import (
+ "database/sql"
+ "errors"
+ "net/http"
+ "strconv"
+ "strings"
+
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+)
+
+// UpdateHandler implements an http handler that updates a server's upd_pending and reval_pending values.
+func UpdateHandler(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"id-or-name"}, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ idOrName := inf.Params["id-or-name"]
+ id, err := strconv.Atoi(idOrName)
+ hostName := ""
+ if err == nil {
+ name, ok, err := dbhelpers.GetServerNameFromID(inf.Tx.Tx, id)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server name from id '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server ID '"+idOrName+"' not found"), nil)
+ return
+ }
+ hostName = name
+ } else {
+ hostName = idOrName
+ if _, ok, err := dbhelpers.GetServerIDFromName(hostName, inf.Tx.Tx); err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server id from name '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server name '"+idOrName+"' not found"), nil)
+ return
+ }
+ }
+
+ updated, hasUpdated := inf.Params["updated"]
+ revalUpdated, hasRevalUpdated := inf.Params["reval_updated"]
+ if !hasUpdated && !hasRevalUpdated {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("Must pass at least one query paramter of 'updated' or 'reval_updated'"), nil)
+ return
+ }
+ updated = strings.ToLower(updated)
+ revalUpdated = strings.ToLower(revalUpdated)
+
+ if hasUpdated && !strings.HasPrefix(updated, "t") && !strings.HasPrefix(updated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'updated' must be 'true' or 'false'"), nil)
+ return
+ }
+ if hasRevalUpdated && !strings.HasPrefix(revalUpdated, "t") && !strings.HasPrefix(revalUpdated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'reval_updated' must be 'true' or 'false'"), nil)
+ return
+ }
+
+ strToBool := func(s string) bool {
+ return !strings.HasPrefix(strings.ToLower(s), "f")
+ }
+
+ updatedPtr := (*bool)(nil)
+ if hasUpdated {
+ updatedBool := strToBool(updated)
+ updatedPtr = &updatedBool
+ }
+ revalUpdatedPtr := (*bool)(nil)
+ if hasRevalUpdated {
+ revalUpdatedBool := strToBool(revalUpdated)
+ revalUpdatedPtr = &revalUpdatedBool
+ }
+
+ if err := setUpdateStatuses(inf.Tx.Tx, hostName, updatedPtr, revalUpdatedPtr); err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("setting updated statuses: "+err.Error()))
+ return
+ }
+
+ err = api.CreateChangeLogBuildMsg(
+ api.ApiChange,
+ api.Updated,
+ inf.User,
+ inf.Tx.Tx,
+ "server-update-status",
+ hostName,
+ map[string]interface{}{"host_name": hostName},
+ )
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("writing changelog: "+err.Error()))
+ return
+ }
+
+ respMsg := "successfully set server '" + hostName + "'"
+ if hasUpdated {
+ respMsg += " updated=" + strconv.FormatBool(strToBool(updated))
+ }
+ if hasRevalUpdated {
+ respMsg += " reval_updated=" + strconv.FormatBool(strToBool(revalUpdated))
+ }
+ api.WriteResp(w, r, respMsg)
Review comment:
Success messages should go in `success`-level alerts, not response objects.
----------------------------------------------------------------
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
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r404258409
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,81 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
+
+*******************************
+``servers/{{HostName-Or-ID}}/update``
+*******************************
Review comment:
underline/overline need to be as long as the title - including formatting characters
----------------------------------------------------------------
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
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r405870604
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,87 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
Review comment:
Duplicate label. This should probably be `to-api-servers-hostname-update` to match the path param name choice used for the file 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] rob05c commented on a change in pull request
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r405846154
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,81 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
+
+*************************************
+``servers/{{HostName-Or-ID}}/update``
+*************************************
+
+``POST``
+========
+:term:`Queue` or dequeue updates and revalidation updates for a specific server.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type: Object
+
+Request Structure
+-----------------
+.. table:: Request Path Parameters
+
+ +------------------+---------------------------------------------------------------------------------------------------------+
+ | Name | Description |
+ +==================+=========================================================================================================+
+ | HostName-OR-ID | The hostName or integral, unique identifier of the server on which updates are being queued or dequeued |
+ +------------------+---------------------------------------------------------------------------------------------------------+
+
+.. table:: Request Query Parameters
+
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | Name | Required | Description |
+ +===============+==========+======================================================================================+
+ | updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | reval_updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
+ +---------------+----------+--------------------------------------------------------------------------------------+
+
+.. code-block:: http
+ :caption: Request Example
+
+ GET /api/2.0/servers/my-edge/update?updated=true&reval_updated=false HTTP/1.1
+ Host: trafficops.infra.ciab.test
+ User-Agent: curl/7.47.0
+ Accept: */*
+ Cookie: mojolicious=...
+
+Response Structure
+------------------
+A human-readable string explaining the value(s) updated.
Review comment:
Done
----------------------------------------------------------------
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
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r404262401
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,81 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
+
+*******************************
+``servers/{{HostName-Or-ID}}/update``
+*******************************
+
+``POST``
+========
+:term:`Queue` or dequeue updates and revalidation updates for a specific server.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type: Object
+
+Request Structure
+-----------------
+.. table:: Request Path Parameters
+
+ +------------------+---------------------------------------------------------------------------------------------------------+
+ | Name | Description |
+ +==================+=========================================================================================================+
+ | HostName-OR-ID | The hostName or integral, unique identifier of the server on which updates are being queued or dequeued |
+ +------------------+---------------------------------------------------------------------------------------------------------+
+
+.. table:: Request Query Parameters
+
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | Name | Required | Description |
+ +===============+==========+======================================================================================+
+ | updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | reval_updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
Review comment:
You don't need to, but it might be nice if this said how it's case-insensitive.
----------------------------------------------------------------
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] rob05c commented on a change in pull request
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r404400826
##########
File path: traffic_ops/traffic_ops_golang/server/update.go
##########
@@ -0,0 +1,155 @@
+package server
+
+/*
+ * 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.
+ */
+
+import (
+ "database/sql"
+ "errors"
+ "net/http"
+ "strconv"
+ "strings"
+
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+)
+
+// UpdateHandler implements an http handler that updates a server's upd_pending and reval_pending values.
+func UpdateHandler(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"id-or-name"}, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ idOrName := inf.Params["id-or-name"]
+ id, err := strconv.Atoi(idOrName)
+ hostName := ""
+ if err == nil {
+ name, ok, err := dbhelpers.GetServerNameFromID(inf.Tx.Tx, id)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server name from id '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server ID '"+idOrName+"' not found"), nil)
+ return
+ }
+ hostName = name
+ } else {
+ hostName = idOrName
+ if _, ok, err := dbhelpers.GetServerIDFromName(hostName, inf.Tx.Tx); err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server id from name '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server name '"+idOrName+"' not found"), nil)
+ return
+ }
+ }
+
+ updated, hasUpdated := inf.Params["updated"]
+ revalUpdated, hasRevalUpdated := inf.Params["reval_updated"]
+ if !hasUpdated && !hasRevalUpdated {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("Must pass at least one query paramter of 'updated' or 'reval_updated'"), nil)
+ return
+ }
+ updated = strings.ToLower(updated)
+ revalUpdated = strings.ToLower(revalUpdated)
+
+ if hasUpdated && !strings.HasPrefix(updated, "t") && !strings.HasPrefix(updated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'updated' must be 'true' or 'false'"), nil)
+ return
+ }
+ if hasRevalUpdated && !strings.HasPrefix(revalUpdated, "t") && !strings.HasPrefix(revalUpdated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'reval_updated' must be 'true' or 'false'"), nil)
+ return
+ }
+
+ strToBool := func(s string) bool {
+ return !strings.HasPrefix(strings.ToLower(s), "f")
+ }
+
+ updatedPtr := (*bool)(nil)
+ if hasUpdated {
+ updatedBool := strToBool(updated)
+ updatedPtr = &updatedBool
+ }
+ revalUpdatedPtr := (*bool)(nil)
+ if hasRevalUpdated {
+ revalUpdatedBool := strToBool(revalUpdated)
+ revalUpdatedPtr = &revalUpdatedBool
+ }
Review comment:
Hm, `0` and `no` feel weird to me. I can see how that would allow some dynamic languages to work with some auto-serialization, but that feels a little _too_ wrong to me. How about just case-insensitive `t`/`true`/`f`/`false`?
----------------------------------------------------------------
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] rob05c commented on a change in pull request
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
rob05c commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r405846937
##########
File path: traffic_ops/testing/api/v1/updatestatus_test.go
##########
@@ -25,6 +25,7 @@ func TestUpdateStatus(t *testing.T) {
CreateTestDeliveryServiceServers(t)
GetTestUpdateStatus(t)
+ GetTestUpdateStatuses(t)
Review comment:
Ah, yeah, that was a mistake, put that there before I realized the test needed to go in /v2, didn't realize it was committed. Fixed.
----------------------------------------------------------------
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
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r404372849
##########
File path: traffic_ops/traffic_ops_golang/server/update.go
##########
@@ -0,0 +1,155 @@
+package server
+
+/*
+ * 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.
+ */
+
+import (
+ "database/sql"
+ "errors"
+ "net/http"
+ "strconv"
+ "strings"
+
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+ "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+)
+
+// UpdateHandler implements an http handler that updates a server's upd_pending and reval_pending values.
+func UpdateHandler(w http.ResponseWriter, r *http.Request) {
+ inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"id-or-name"}, nil)
+ if userErr != nil || sysErr != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, errCode, userErr, sysErr)
+ return
+ }
+ defer inf.Close()
+
+ idOrName := inf.Params["id-or-name"]
+ id, err := strconv.Atoi(idOrName)
+ hostName := ""
+ if err == nil {
+ name, ok, err := dbhelpers.GetServerNameFromID(inf.Tx.Tx, id)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server name from id '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server ID '"+idOrName+"' not found"), nil)
+ return
+ }
+ hostName = name
+ } else {
+ hostName = idOrName
+ if _, ok, err := dbhelpers.GetServerIDFromName(hostName, inf.Tx.Tx); err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting server id from name '"+idOrName+"': "+err.Error()))
+ return
+ } else if !ok {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("server name '"+idOrName+"' not found"), nil)
+ return
+ }
+ }
+
+ updated, hasUpdated := inf.Params["updated"]
+ revalUpdated, hasRevalUpdated := inf.Params["reval_updated"]
+ if !hasUpdated && !hasRevalUpdated {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("Must pass at least one query paramter of 'updated' or 'reval_updated'"), nil)
+ return
+ }
+ updated = strings.ToLower(updated)
+ revalUpdated = strings.ToLower(revalUpdated)
+
+ if hasUpdated && !strings.HasPrefix(updated, "t") && !strings.HasPrefix(updated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'updated' must be 'true' or 'false'"), nil)
+ return
+ }
+ if hasRevalUpdated && !strings.HasPrefix(revalUpdated, "t") && !strings.HasPrefix(revalUpdated, "f") {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("query parameter 'reval_updated' must be 'true' or 'false'"), nil)
+ return
+ }
+
+ strToBool := func(s string) bool {
+ return !strings.HasPrefix(strings.ToLower(s), "f")
+ }
+
+ updatedPtr := (*bool)(nil)
+ if hasUpdated {
+ updatedBool := strToBool(updated)
+ updatedPtr = &updatedBool
+ }
+ revalUpdatedPtr := (*bool)(nil)
+ if hasRevalUpdated {
+ revalUpdatedBool := strToBool(revalUpdated)
+ revalUpdatedPtr = &revalUpdatedBool
+ }
Review comment:
That's a bit too permissive. Accepting `False`, `false`, `FALSE`, and even `f`/`F` equally makes sense, and even `0`, or `no`, but accepting `ftrue` as `false` is a bridge too far. That could've just been a typo, and in fact probably was.
----------------------------------------------------------------
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
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r405700053
##########
File path: traffic_ops/testing/api/v1/updatestatus_test.go
##########
@@ -25,6 +25,7 @@ func TestUpdateStatus(t *testing.T) {
CreateTestDeliveryServiceServers(t)
GetTestUpdateStatus(t)
+ GetTestUpdateStatuses(t)
Review comment:
> `undefined: GetTestUpdateStatuses`
----------------------------------------------------------------
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
#4599: Add Traffic Ops /server/update
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #4599: Add Traffic Ops /server/update
URL: https://github.com/apache/trafficcontrol/pull/4599#discussion_r404370969
##########
File path: docs/source/api/v2/servers_hostname_update.rst
##########
@@ -0,0 +1,81 @@
+..
+..
+.. Licensed 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.
+..
+
+.. _to-api-servers-id-queue_update:
+
+*******************************
+``servers/{{HostName-Or-ID}}/update``
+*******************************
+
+``POST``
+========
+:term:`Queue` or dequeue updates and revalidation updates for a specific server.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type: Object
+
+Request Structure
+-----------------
+.. table:: Request Path Parameters
+
+ +------------------+---------------------------------------------------------------------------------------------------------+
+ | Name | Description |
+ +==================+=========================================================================================================+
+ | HostName-OR-ID | The hostName or integral, unique identifier of the server on which updates are being queued or dequeued |
+ +------------------+---------------------------------------------------------------------------------------------------------+
+
+.. table:: Request Query Parameters
+
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | Name | Required | Description |
+ +===============+==========+======================================================================================+
+ | updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
+ +---------------+----------+--------------------------------------------------------------------------------------+
+ | reval_updated | no | The value to set for the queue update flag on this server. May be 'true' or 'false'. |
Review comment:
No, that's perfectly fine as far as I'm concerned.
----------------------------------------------------------------
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