You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2020/05/20 07:29:48 UTC

[GitHub] [servicecomb-java-chassis] wujimin opened a new pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

wujimin opened a new pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767


   


----------------------------------------------------------------
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] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767#discussion_r428391942



##########
File path: swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/context/InvocationContext.java
##########
@@ -36,6 +38,11 @@
 
   protected Map<String, Object> localContext = new HashMap<>();
 
+  /**
+   * the vertx context that start the invocation
+   */
+  protected Context vertxContext;

Review comment:
       How about this ?
   
   ```
   Interface TransportContext
   
   class VertxTransportContext implments TransportContext 
       {Context vertxContext; RoutingContext routingContext; ...}
   class ServletTransportContext implements TransportContext
       {HttpServletRequest rquest; HttpServletResponse response;}
   ```




----------------------------------------------------------------
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] [servicecomb-java-chassis] wujimin commented on a change in pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

Posted by GitBox <gi...@apache.org>.
wujimin commented on a change in pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767#discussion_r428994393



##########
File path: swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/context/InvocationContext.java
##########
@@ -36,6 +38,11 @@
 
   protected Map<String, Object> localContext = new HashMap<>();
 
+  /**
+   * the vertx context that start the invocation
+   */
+  protected Context vertxContext;

Review comment:
       maybe useful for new Filter mechanism, let me try it.




----------------------------------------------------------------
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] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767#discussion_r429016600



##########
File path: swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/context/InvocationContext.java
##########
@@ -36,6 +38,11 @@
 
   protected Map<String, Object> localContext = new HashMap<>();
 
+  /**
+   * the vertx context that start the invocation
+   */
+  protected Context vertxContext;

Review comment:
       OK. New filter mechanism should consider a 'global error handler'(errors thrown everywhere) .  This is a frequently asked questions and java chassis not handle it well now. 




----------------------------------------------------------------
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] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767#discussion_r428391942



##########
File path: swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/context/InvocationContext.java
##########
@@ -36,6 +38,11 @@
 
   protected Map<String, Object> localContext = new HashMap<>();
 
+  /**
+   * the vertx context that start the invocation
+   */
+  protected Context vertxContext;

Review comment:
       How about this ?
   
   ```
   Interface TransportContext
   
   interface TransportProviderContext extends TransportContext
   interface TransportConsumerContext extends TransportContext
   
   class VertxTransportProviderContext implments TransportContext 
       {Context vertxContext; RoutingContext routingContext; ...}
   class ServletTransportProviderContext implements TransportContext
       {HttpServletRequest rquest; HttpServletResponse response;}
   ```
   
   




----------------------------------------------------------------
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] [servicecomb-java-chassis] coveralls edited a comment on pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767#issuecomment-631314444


   
   [![Coverage Status](https://coveralls.io/builds/30961538/badge)](https://coveralls.io/builds/30961538)
   
   Coverage decreased (-0.02%) to 84.575% when pulling **3f345c9f132b6b4467aa19acc332fdd6bd8c0586 on wujimin:SCB-1924-add-vertx-context-to-invocation** into **c8d75ea44fb45b528d111956092b634d6a7dd2e9 on apache:master**.
   


----------------------------------------------------------------
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] [servicecomb-java-chassis] wujimin commented on a change in pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

Posted by GitBox <gi...@apache.org>.
wujimin commented on a change in pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767#discussion_r429035953



##########
File path: swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/context/InvocationContext.java
##########
@@ -36,6 +38,11 @@
 
   protected Map<String, Object> localContext = new HashMap<>();
 
+  /**
+   * the vertx context that start the invocation
+   */
+  protected Context vertxContext;

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

[GitHub] [servicecomb-java-chassis] wujimin commented on a change in pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

Posted by GitBox <gi...@apache.org>.
wujimin commented on a change in pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767#discussion_r429028255



##########
File path: swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/context/InvocationContext.java
##########
@@ -36,6 +38,11 @@
 
   protected Map<String, Object> localContext = new HashMap<>();
 
+  /**
+   * the vertx context that start the invocation
+   */
+  protected Context vertxContext;

Review comment:
       yes, already design it.




----------------------------------------------------------------
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] [servicecomb-java-chassis] liubao68 merged pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

Posted by GitBox <gi...@apache.org>.
liubao68 merged pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767


   


----------------------------------------------------------------
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] [servicecomb-java-chassis] wujimin commented on a change in pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

Posted by GitBox <gi...@apache.org>.
wujimin commented on a change in pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767#discussion_r427921194



##########
File path: swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/context/InvocationContext.java
##########
@@ -36,6 +38,11 @@
 
   protected Map<String, Object> localContext = new HashMap<>();
 
+  /**
+   * the vertx context that start the invocation
+   */
+  protected Context vertxContext;

Review comment:
       seems useless to record context for web container?  
   and i don't know what is the context for web container, 😂




----------------------------------------------------------------
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] [servicecomb-java-chassis] wujimin commented on a change in pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

Posted by GitBox <gi...@apache.org>.
wujimin commented on a change in pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767#discussion_r428394126



##########
File path: swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/context/InvocationContext.java
##########
@@ -36,6 +38,11 @@
 
   protected Map<String, Object> localContext = new HashMap<>();
 
+  /**
+   * the vertx context that start the invocation
+   */
+  protected Context vertxContext;

Review comment:
       request already can be got from invocation.  
   and not want publish servlet response......  




----------------------------------------------------------------
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] [servicecomb-java-chassis] coveralls commented on pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767#issuecomment-631314444


   
   [![Coverage Status](https://coveralls.io/builds/30911866/badge)](https://coveralls.io/builds/30911866)
   
   Coverage decreased (-0.01%) to 85.112% when pulling **a1cf96b193dcfc7c5cd3b342c67930e0882dd40a on wujimin:SCB-1924-add-vertx-context-to-invocation** into **55ec91e22bf668a5f1b9785837ff8d97af7794f0 on apache:master**.
   


----------------------------------------------------------------
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] [servicecomb-java-chassis] liubao68 commented on a change in pull request #1767: [SCB-1924] invocation context provide the vertx context that start the invocation

Posted by GitBox <gi...@apache.org>.
liubao68 commented on a change in pull request #1767:
URL: https://github.com/apache/servicecomb-java-chassis/pull/1767#discussion_r427901534



##########
File path: swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/context/InvocationContext.java
##########
@@ -36,6 +38,11 @@
 
   protected Map<String, Object> localContext = new HashMap<>();
 
+  /**
+   * the vertx context that start the invocation
+   */
+  protected Context vertxContext;

Review comment:
       How about `T transportContext` ? Because vert.x or web container have different context. 




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