[GitHub] rabbah commented on a change in pull request #2591: User-Agent CLI/version header

2017-09-01 Thread git 
rabbah commented on a change in pull request #2591: User-Agent CLI/version 
header
URL: 
https://github.com/apache/incubator-openwhisk/pull/2591#discussion_r136647084
 
 

 ##
 File path: tests/src/test/scala/whisk/core/cli/test/WskBasicUsageTests.scala
 ##
 @@ -120,7 +120,7 @@ class WskBasicUsageTests
 }
 
 it should "include CLI user agent headers with outbound requests" in {
-val stdout = wsk.cli(Seq("list"), verbose = true).stdout
+val stdout = wsk.cli(Seq("list", "--auth", wskprops.authKey) ++ 
wskprops.overrides, verbose = true).stdout
 stdout should include regex ("""\bUser-Agent\b": 
\[\s+"CloudFunctions\-CLI/1.\d+.*""")
 
 Review comment:
   CloudFunctions?
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #2591: User-Agent CLI/version header

2017-08-31 Thread git 
rabbah commented on a change in pull request #2591: User-Agent CLI/version 
header
URL: 
https://github.com/apache/incubator-openwhisk/pull/2591#discussion_r136485499
 
 

 ##
 File path: tests/src/test/scala/whisk/core/cli/test/WskBasicUsageTests.scala
 ##
 @@ -119,6 +119,10 @@ class WskBasicUsageTests
 wsk.action.get(fullQualifiedName).stdout should include(s"ok: got 
action ${packageName}/${actionName}")
 }
 
+it should "include CLI user agent headers with outbound requests" in {
+val stdout = wsk.action.list(verbose = true).stdout
+stdout should include regex ("""\bUser-Agent\b": 
\[\s+"OpenWhisk\-CLI/1.\d+.*""")
 
 Review comment:
   since the goal is just to run a cli command with verbose on, i'd rather you 
don't change the `list` method and install use `wsk.cli(Seq("list"), verbose = 
true)` directly.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #2591: User-Agent CLI/version header

2017-08-16 Thread git 
rabbah commented on a change in pull request #2591: User-Agent CLI/version 
header
URL: 
https://github.com/apache/incubator-openwhisk/pull/2591#discussion_r133610341
 
 

 ##
 File path: docs/rest_api.md
 ##
 @@ -80,11 +80,14 @@ wsk namespace list -v
 ```
 ```
 REQUEST:
-[GET]  https://openwhisk.ng.bluemix.net/api/v1/namespaces
+[GET]  https://openwhisk.ng.bluemix.net/api/v1/namespaces
 Req Headers
 {
   "Authorization": [
 "Basic XXX"
+  ],
+  "User-Agent": [
 
 Review comment:
   back out changes to this file?
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #2591: User-Agent CLI/version header

2017-08-11 Thread git 
rabbah commented on a change in pull request #2591: User-Agent CLI/version 
header
URL: 
https://github.com/apache/incubator-openwhisk/pull/2591#discussion_r132715559
 
 

 ##
 File path: core/controller/src/main/scala/whisk/core/controller/RestAPIs.scala
 ##
 @@ -121,7 +121,7 @@ protected[controller] object RestApiCommons {
  */
 protected[controller] trait RespondWithHeaders extends Directives {
 val allowOrigin = `Access-Control-Allow-Origin`(AllOrigins)
-val allowHeaders = `Access-Control-Allow-Headers`("Authorization", 
"Content-Type")
+val allowHeaders = `Access-Control-Allow-Headers`("Authorization", 
"Content-Type", "User-Agent")
 
 Review comment:
   the controller does not log user agents - i don't think it should/needs to.
   you get that from nginx.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #2591: User-Agent CLI/version header

2017-08-11 Thread git 
rabbah commented on a change in pull request #2591: User-Agent CLI/version 
header
URL: 
https://github.com/apache/incubator-openwhisk/pull/2591#discussion_r132715559
 
 

 ##
 File path: core/controller/src/main/scala/whisk/core/controller/RestAPIs.scala
 ##
 @@ -121,7 +121,7 @@ protected[controller] object RestApiCommons {
  */
 protected[controller] trait RespondWithHeaders extends Directives {
 val allowOrigin = `Access-Control-Allow-Origin`(AllOrigins)
-val allowHeaders = `Access-Control-Allow-Headers`("Authorization", 
"Content-Type")
+val allowHeaders = `Access-Control-Allow-Headers`("Authorization", 
"Content-Type", "User-Agent")
 
 Review comment:
   the controller does not log user agent or any other header - i don't think 
it should/needs to.
   you get that from nginx.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #2591: User-Agent CLI/version header

2017-08-11 Thread git 
rabbah commented on a change in pull request #2591: User-Agent CLI/version 
header
URL: 
https://github.com/apache/incubator-openwhisk/pull/2591#discussion_r132698730
 
 

 ##
 File path: 
core/controller/src/main/scala/whisk/core/controller/WebActions.scala
 ##
 @@ -326,7 +326,7 @@ trait WhiskWebActionsApi
 private val defaultCorsResponse = List(
 `Access-Control-Allow-Origin`(AllOrigins),
 `Access-Control-Allow-Methods`(OPTIONS, GET, DELETE, POST, PUT, HEAD, 
PATCH),
-`Access-Control-Allow-Headers`(`Authorization`.name, 
`Content-Type`.name))
+`Access-Control-Allow-Headers`(`Authorization`.name, 
`Content-Type`.name, `User-Agent`.name))
 
 Review comment:
   But the controller does nothing with these user agents for POST. And 
accepting it here for web actions is just for preflight - I suppose it's fine 
(but for the cli this has no value there). If you want to keep it I think we 
need a doc update. 
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #2591: User-Agent CLI/version header

2017-08-10 Thread git 
rabbah commented on a change in pull request #2591: User-Agent CLI/version 
header
URL: 
https://github.com/apache/incubator-openwhisk/pull/2591#discussion_r132605304
 
 

 ##
 File path: core/controller/src/main/scala/whisk/core/controller/RestAPIs.scala
 ##
 @@ -121,7 +121,7 @@ protected[controller] object RestApiCommons {
  */
 protected[controller] trait RespondWithHeaders extends Directives {
 val allowOrigin = `Access-Control-Allow-Origin`(AllOrigins)
-val allowHeaders = `Access-Control-Allow-Headers`("Authorization", 
"Content-Type")
+val allowHeaders = `Access-Control-Allow-Headers`("Authorization", 
"Content-Type", "User-Agent")
 
 Review comment:
   what is the value of the controller accepting the user-agent header?
   should nginx just drop this header?
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #2591: User-Agent CLI/version header

2017-08-10 Thread git 
rabbah commented on a change in pull request #2591: User-Agent CLI/version 
header
URL: 
https://github.com/apache/incubator-openwhisk/pull/2591#discussion_r132605285
 
 

 ##
 File path: 
core/controller/src/main/scala/whisk/core/controller/WebActions.scala
 ##
 @@ -326,7 +326,7 @@ trait WhiskWebActionsApi
 private val defaultCorsResponse = List(
 `Access-Control-Allow-Origin`(AllOrigins),
 `Access-Control-Allow-Methods`(OPTIONS, GET, DELETE, POST, PUT, HEAD, 
PATCH),
-`Access-Control-Allow-Headers`(`Authorization`.name, 
`Content-Type`.name))
+`Access-Control-Allow-Headers`(`Authorization`.name, 
`Content-Type`.name, `User-Agent`.name))
 
 Review comment:
   do we want this for a web action?
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rabbah commented on a change in pull request #2591: User-Agent CLI/version header

2017-08-10 Thread git 
rabbah commented on a change in pull request #2591: User-Agent CLI/version 
header
URL: 
https://github.com/apache/incubator-openwhisk/pull/2591#discussion_r132605304
 
 

 ##
 File path: core/controller/src/main/scala/whisk/core/controller/RestAPIs.scala
 ##
 @@ -121,7 +121,7 @@ protected[controller] object RestApiCommons {
  */
 protected[controller] trait RespondWithHeaders extends Directives {
 val allowOrigin = `Access-Control-Allow-Origin`(AllOrigins)
-val allowHeaders = `Access-Control-Allow-Headers`("Authorization", 
"Content-Type")
+val allowHeaders = `Access-Control-Allow-Headers`("Authorization", 
"Content-Type", "User-Agent")
 
 Review comment:
   what is the value of the controller accepting the user-agent?
   should nginx just drop this header?
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services