You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/06/21 06:27:53 UTC

[GitHub] [incubator-apisix] shenal opened a new pull request #1746: Added content-type admin API responses

shenal opened a new pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746


   ### Summary
   
   SUMMARY_HERE
   This PR adds `Content-Type: application/json` to the Admin API response headers fixing this [issue](https://github.com/apache/incubator-apisix/issues/1745)
   
   ### Issues resolved 
   
   Fix #1745 
   


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

[GitHub] [incubator-apisix] shenal commented on a change in pull request #1746: Added content-type admin API responses

Posted by GitBox <gi...@apache.org>.
shenal commented on a change in pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746#discussion_r444645383



##########
File path: t/admin/routes.t
##########
@@ -321,6 +328,7 @@ GET /t
                 }]]
                 )
 
+            ngx.header["Content Type"] = res.header["Content Type"]

Review comment:
       I think this issue has been already resolved in Openresty




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

[GitHub] [incubator-apisix] juzhiyuan commented on a change in pull request #1746: Added content-type admin API responses

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746#discussion_r444621764



##########
File path: t/admin/routes.t
##########
@@ -321,6 +328,7 @@ GET /t
                 }]]
                 )
 
+            ngx.header["Content Type"] = res.header["Content Type"]

Review comment:
       ![image](https://user-images.githubusercontent.com/2106987/85495012-529b6580-b60c-11ea-80c0-12aac7e929b4.png)
   




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

[GitHub] [incubator-apisix] membphis commented on pull request #1746: Added content-type admin API responses

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746#issuecomment-647097230


   please add some test cases about this.


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

[GitHub] [incubator-apisix] membphis commented on pull request #1746: Added content-type admin API responses

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746#issuecomment-648550983


   @shenal I suggest not to modify any existing test cases, you can add new ones for testing.


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

[GitHub] [incubator-apisix] shenal commented on pull request #1746: Added content-type admin API responses

Posted by GitBox <gi...@apache.org>.
shenal commented on pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746#issuecomment-648551967


   > @shenal I suggest not to modify any existing test cases, you can add new ones for testing.
   > This will make your PR simpler.
   
   @membphis Noted. I was investigating the test suite and found out that the test client defined in `test_admin.lua` does not return the response headers for me to assert. I am looking for a different approach


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

[GitHub] [incubator-apisix] membphis commented on pull request #1746: Added content-type admin API responses

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746#issuecomment-647307565


   @shenal pls take a look at the output of Travis-CI:
   
   ```
   Test Summary Report
   -------------------
   t/admin/routes.t                     (Wstat: 1024 Tests: 150 Failed: 4)
     Failed tests:  2, 15, 19, 23
     Non-zero exit status: 4
   Files=116, Tests=3847, 882 wallclock secs ( 1.13 usr  0.12 sys + 120.55 cusr 21.98 csys = 143.78 CPU)
   Result: FAIL
   The command "$PWD/.travis/${OSNAME}_runner.sh script" exited with 1.
   ```


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

[GitHub] [incubator-apisix] membphis commented on pull request #1746: Added content-type admin API responses

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746#issuecomment-648560077


   @shenal if you need any help, please let us know. ^_^


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

[GitHub] [incubator-apisix] membphis merged pull request #1746: Added content-type admin API responses

Posted by GitBox <gi...@apache.org>.
membphis merged pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746


   


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

[GitHub] [incubator-apisix] membphis commented on pull request #1746: Added content-type admin API responses

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746#issuecomment-648607754


   @shenal many thx


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

[GitHub] [incubator-apisix] membphis edited a comment on pull request #1746: Added content-type admin API responses

Posted by GitBox <gi...@apache.org>.
membphis edited a comment on pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746#issuecomment-648550983


   @shenal I suggest not to modify any existing test cases, you can add new ones for testing.
   This will make your PR simpler.


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

[GitHub] [incubator-apisix] shenal commented on pull request #1746: Added content-type admin API responses

Posted by GitBox <gi...@apache.org>.
shenal commented on pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746#issuecomment-648577894


   > @shenal if you need any help, please let us know. ^_^
   
   Thanks @membphis  appreciate it :smiley: . This issue made me dig more into the internals of the `lua-resty-http` client. I think the tests should pass now :crossed_fingers: 


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

[GitHub] [incubator-apisix] shenal commented on pull request #1746: Added content-type admin API responses

Posted by GitBox <gi...@apache.org>.
shenal commented on pull request #1746:
URL: https://github.com/apache/incubator-apisix/pull/1746#issuecomment-648599564


   @membphis FYI The tests have passed. Third time was the charm


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