You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@submarine.apache.org by GitBox <gi...@apache.org> on 2020/04/28 11:24:42 UTC

[GitHub] [submarine] pingsutw opened a new pull request #271: SUBMARINE-481. Use Swagger to describe and document submarine RESTful APIs

pingsutw opened a new pull request #271:
URL: https://github.com/apache/submarine/pull/271


   ### What is this PR for?
   Use swagger annotation to generate API document 
   https://gist.github.com/pingsutw/4d96afb54d4c767304f74844b432585c
   1. Build Submarine source code
   2. start submarine server
   3. open localhost:8080/v2/swagger.json
   
   ### What type of PR is it?
   [Improvement]
   
   ### Todos
   * [ ] - Task
   
   ### What is the Jira issue?
   https://issues.apache.org/jira/browse/SUBMARINE-481
   
   ### How should this be tested?
   https://travis-ci.org/github/pingsutw/hadoop-submarine/builds/680491536
   
   ### Screenshots (if appropriate)
   
   ### Questions:
   * Does the licenses files need update? No
   * Is there breaking changes for older versions? No
   * Does this needs documentation? No
   


----------------------------------------------------------------
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] [submarine] jiwq commented on a change in pull request #271: SUBMARINE-481. Use Swagger to describe and document submarine RESTful APIs

Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #271:
URL: https://github.com/apache/submarine/pull/271#discussion_r416590349



##########
File path: submarine-server/server-core/pom.xml
##########
@@ -38,6 +38,46 @@
       <version>${project.version}</version>
     </dependency>
 
+    <dependency>
+      <groupId>io.swagger</groupId>
+      <artifactId>swagger-jersey2-jaxrs</artifactId>
+      <version>1.5.18</version>

Review comment:
       Should define the version in properties.




----------------------------------------------------------------
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] [submarine] jiwq commented on a change in pull request #271: SUBMARINE-481. Use Swagger to describe and document submarine RESTful APIs

Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #271:
URL: https://github.com/apache/submarine/pull/271#discussion_r416590555



##########
File path: submarine-server/server-core/pom.xml
##########
@@ -38,6 +38,46 @@
       <version>${project.version}</version>
     </dependency>
 
+    <dependency>
+      <groupId>io.swagger</groupId>
+      <artifactId>swagger-jersey2-jaxrs</artifactId>
+      <version>1.5.18</version>

Review comment:
       Should define the version in properties.

##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/Bootstrap.java
##########
@@ -0,0 +1,36 @@
+package org.apache.submarine.server;

Review comment:
       Missing the license header.

##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/Bootstrap.java
##########
@@ -0,0 +1,36 @@
+package org.apache.submarine.server;
+
+import io.swagger.jaxrs.config.SwaggerContextService;
+import io.swagger.models.Contact;
+import io.swagger.models.Info;
+import io.swagger.models.License;
+import io.swagger.models.Swagger;
+
+

Review comment:
       Redundant blank line.

##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/rest/JobManagerRestApi.java
##########
@@ -38,9 +38,15 @@
 import org.apache.submarine.server.api.spec.JobSpec;
 import org.apache.submarine.server.response.JsonResponse;
 
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+
 /**
  * Job Service REST API v1. It can accept {@link JobSpec} to create a job.
  */
+@Api(description = "the jobs API")

Review comment:
       ```suggestion
   @Api(description = "The Job REST API")
   ```

##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/Bootstrap.java
##########
@@ -0,0 +1,36 @@
+package org.apache.submarine.server;
+
+import io.swagger.jaxrs.config.SwaggerContextService;
+import io.swagger.models.Contact;
+import io.swagger.models.Info;
+import io.swagger.models.License;
+import io.swagger.models.Swagger;
+
+
+import javax.servlet.http.HttpServlet;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletConfig;
+import javax.servlet.ServletException;
+
+public class Bootstrap extends HttpServlet {
+  @Override
+  public void init(ServletConfig config) throws ServletException {
+    Info info = new Info()
+            .title("Swagger Server")
+            .description("The Submarine REST API allows you to create, list, and get experiments." +
+                    " The API is hosted under the /v1/jobs route on the Submarine server. For example," +
+                    " to list experiments on a server hosted at http://localhost:8080" +
+                    ", access http://localhost:8080/api/v1/jobs/status")
+            .termsOfService("http://swagger.io/terms/")
+            .contact(new Contact()
+                    .email("submarine-dev@submarine.apache.org"))

Review comment:
       ```suggestion
                       .email("dev@submarine.apache.org"))
   ```

##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/rest/JobManagerRestApi.java
##########
@@ -123,6 +151,11 @@ public Response patchJob(@PathParam(RestConstants.JOB_ID) String id, JobSpec spe
    */
   @DELETE
   @Path("/{id}")
+  @ApiOperation(value = "Deletes a job", response = Job.class, tags = { "jobs"})

Review comment:
       ```suggestion
     @ApiOperation(value = "Delete the job", response = Job.class, tags = { "jobs"})
   ```

##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/rest/JobManagerRestApi.java
##########
@@ -64,6 +71,10 @@ public Response ping() {
    */
   @POST
   @Consumes({RestConstants.MEDIA_TYPE_YAML, MediaType.APPLICATION_JSON})
+  @ApiOperation(value = "Submit a job to server", response = Job.class, tags = { "jobs"})

Review comment:
       ```suggestion
     @ApiOperation(value = "Create a job", response = Job.class, tags = { "jobs"})
   ```

##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/rest/JobManagerRestApi.java
##########
@@ -106,6 +129,11 @@ public Response getJob(@PathParam(RestConstants.JOB_ID) String id) {
   @PATCH
   @Path("/{id}")
   @Consumes({RestConstants.MEDIA_TYPE_YAML, MediaType.APPLICATION_JSON})
+  @ApiOperation(value = "Updates a job in the submarine server with job spec",

Review comment:
       ```suggestion
     @ApiOperation(value = "Update the job in submarine server with job spec",
   ```

##########
File path: submarine-workbench/workbench-web-ng/src/WEB-INF/web.xml
##########
@@ -28,10 +28,34 @@
     <servlet-class>org.glassfish.jersey.servlet.ServletContainer</servlet-class>
     <init-param>
       <param-name>jersey.config.server.provider.packages</param-name>
-      <param-value>org.apache.submarine.server.rest</param-value>
+      <param-value>io.swagger.jaxrs.listing,org.apache.submarine.server.rest</param-value>
     </init-param>
     <load-on-startup>1</load-on-startup>
   </servlet>
+  <servlet>
+    <servlet-name>Jersey2Config</servlet-name>
+    <servlet-class>io.swagger.jersey.config.JerseyJaxrsConfig</servlet-class>
+    <init-param>
+      <param-name>api.version</param-name>
+      <param-value>1.0.0</param-value>
+    </init-param>
+    <init-param>
+      <param-name>swagger.api.basepath</param-name>
+      <param-value>http://localhost:8080/api/</param-value>
+    </init-param>
+    <load-on-startup>2</load-on-startup>
+  </servlet>
+  <servlet>
+    <servlet-name>Bootstrap</servlet-name>
+    <servlet-class>org.apache.submarine.server.Bootstrap</servlet-class>
+    <load-on-startup>2</load-on-startup>
+  </servlet>
+
+  <servlet-mapping>
+    <servlet-name>default</servlet-name>
+    <url-pattern>/v2/*</url-pattern>

Review comment:
       why use the `v2`?

##########
File path: submarine-server/server-core/src/main/java/org/apache/submarine/server/rest/JobManagerRestApi.java
##########
@@ -94,6 +111,12 @@ public Response listJob(@QueryParam("status") String status) {
    */
   @GET
   @Path("/{id}")
+  @ApiOperation(value = "Find job by ID", notes = "Returns a single job",

Review comment:
       ```suggestion
     @ApiOperation(value = "Find job by id", notes = "Return a single job",
   ```




----------------------------------------------------------------
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] [submarine] jiwq commented on a change in pull request #271: SUBMARINE-481. Use Swagger to describe and document submarine RESTful APIs

Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #271:
URL: https://github.com/apache/submarine/pull/271#discussion_r417308220



##########
File path: submarine-workbench/workbench-web-ng/src/WEB-INF/web.xml
##########
@@ -28,10 +28,34 @@
     <servlet-class>org.glassfish.jersey.servlet.ServletContainer</servlet-class>
     <init-param>
       <param-name>jersey.config.server.provider.packages</param-name>
-      <param-value>org.apache.submarine.server.rest</param-value>
+      <param-value>io.swagger.jaxrs.listing,org.apache.submarine.server.rest</param-value>
     </init-param>
     <load-on-startup>1</load-on-startup>
   </servlet>
+  <servlet>
+    <servlet-name>Jersey2Config</servlet-name>
+    <servlet-class>io.swagger.jersey.config.JerseyJaxrsConfig</servlet-class>
+    <init-param>
+      <param-name>api.version</param-name>
+      <param-value>1.0.0</param-value>
+    </init-param>
+    <init-param>
+      <param-name>swagger.api.basepath</param-name>
+      <param-value>http://localhost:8080/api/</param-value>
+    </init-param>
+    <load-on-startup>2</load-on-startup>
+  </servlet>
+  <servlet>
+    <servlet-name>Bootstrap</servlet-name>
+    <servlet-class>org.apache.submarine.server.Bootstrap</servlet-class>
+    <load-on-startup>2</load-on-startup>
+  </servlet>
+
+  <servlet-mapping>
+    <servlet-name>default</servlet-name>
+    <url-pattern>/v2/*</url-pattern>

Review comment:
       I think it should keep align with the version of submarine REST api, at now it used `v1`.




----------------------------------------------------------------
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] [submarine] tangzhankun commented on pull request #271: SUBMARINE-481. Use Swagger to describe and document submarine RESTful APIs

Posted by GitBox <gi...@apache.org>.
tangzhankun commented on pull request #271:
URL: https://github.com/apache/submarine/pull/271#issuecomment-620972709


   > @kevin85421 thanks for the contribution. I'm wondering is it better to support the swagger UI for API document in this PR too?
   
   Let's enable the swagger UI later.


----------------------------------------------------------------
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] [submarine] pingsutw commented on pull request #271: SUBMARINE-481. Use Swagger to describe and document submarine RESTful APIs

Posted by GitBox <gi...@apache.org>.
pingsutw commented on pull request #271:
URL: https://github.com/apache/submarine/pull/271#issuecomment-621645950


   I have attached the experiment.yaml and simple doc about experiments spec in [jira](https://issues.apache.org/jira/browse/SUBMARINE-481), feel free to comment and edit.
   
   


----------------------------------------------------------------
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] [submarine] jiwq commented on a change in pull request #271: SUBMARINE-481. Use Swagger to describe and document submarine RESTful APIs

Posted by GitBox <gi...@apache.org>.
jiwq commented on a change in pull request #271:
URL: https://github.com/apache/submarine/pull/271#discussion_r416590349



##########
File path: submarine-server/server-core/pom.xml
##########
@@ -38,6 +38,46 @@
       <version>${project.version}</version>
     </dependency>
 
+    <dependency>
+      <groupId>io.swagger</groupId>
+      <artifactId>swagger-jersey2-jaxrs</artifactId>
+      <version>1.5.18</version>

Review comment:
       Should define the version in properties.




----------------------------------------------------------------
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] [submarine] tangzhankun commented on a change in pull request #271: SUBMARINE-481. Use Swagger to describe and document submarine RESTful APIs

Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #271:
URL: https://github.com/apache/submarine/pull/271#discussion_r417043743



##########
File path: submarine-workbench/workbench-web-ng/src/WEB-INF/web.xml
##########
@@ -28,10 +28,34 @@
     <servlet-class>org.glassfish.jersey.servlet.ServletContainer</servlet-class>
     <init-param>
       <param-name>jersey.config.server.provider.packages</param-name>
-      <param-value>org.apache.submarine.server.rest</param-value>
+      <param-value>io.swagger.jaxrs.listing,org.apache.submarine.server.rest</param-value>
     </init-param>
     <load-on-startup>1</load-on-startup>
   </servlet>
+  <servlet>
+    <servlet-name>Jersey2Config</servlet-name>
+    <servlet-class>io.swagger.jersey.config.JerseyJaxrsConfig</servlet-class>
+    <init-param>
+      <param-name>api.version</param-name>
+      <param-value>1.0.0</param-value>
+    </init-param>
+    <init-param>
+      <param-name>swagger.api.basepath</param-name>
+      <param-value>http://localhost:8080/api/</param-value>
+    </init-param>
+    <load-on-startup>2</load-on-startup>
+  </servlet>
+  <servlet>
+    <servlet-name>Bootstrap</servlet-name>
+    <servlet-class>org.apache.submarine.server.Bootstrap</servlet-class>
+    <load-on-startup>2</load-on-startup>
+  </servlet>
+
+  <servlet-mapping>
+    <servlet-name>default</servlet-name>
+    <url-pattern>/v2/*</url-pattern>

Review comment:
       do you mean openAPI v3?




----------------------------------------------------------------
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] [submarine] tangzhankun edited a comment on pull request #271: SUBMARINE-481. Use Swagger to describe and document submarine RESTful APIs

Posted by GitBox <gi...@apache.org>.
tangzhankun edited a comment on pull request #271:
URL: https://github.com/apache/submarine/pull/271#issuecomment-620967657


   @kevin85421 thanks for the contribution. I'm wondering is it better to support the swagger UI for API document in this PR too?


----------------------------------------------------------------
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] [submarine] tangzhankun commented on pull request #271: SUBMARINE-481. Use Swagger to describe and document submarine RESTful APIs

Posted by GitBox <gi...@apache.org>.
tangzhankun commented on pull request #271:
URL: https://github.com/apache/submarine/pull/271#issuecomment-620967657


   @kevin85421 thanks for the contribution. I'm wondering is it better to start a swagger API document in this PR too?


----------------------------------------------------------------
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] [submarine] tangzhankun commented on a change in pull request #271: SUBMARINE-481. Use Swagger to describe and document submarine RESTful APIs

Posted by GitBox <gi...@apache.org>.
tangzhankun commented on a change in pull request #271:
URL: https://github.com/apache/submarine/pull/271#discussion_r417043743



##########
File path: submarine-workbench/workbench-web-ng/src/WEB-INF/web.xml
##########
@@ -28,10 +28,34 @@
     <servlet-class>org.glassfish.jersey.servlet.ServletContainer</servlet-class>
     <init-param>
       <param-name>jersey.config.server.provider.packages</param-name>
-      <param-value>org.apache.submarine.server.rest</param-value>
+      <param-value>io.swagger.jaxrs.listing,org.apache.submarine.server.rest</param-value>
     </init-param>
     <load-on-startup>1</load-on-startup>
   </servlet>
+  <servlet>
+    <servlet-name>Jersey2Config</servlet-name>
+    <servlet-class>io.swagger.jersey.config.JerseyJaxrsConfig</servlet-class>
+    <init-param>
+      <param-name>api.version</param-name>
+      <param-value>1.0.0</param-value>
+    </init-param>
+    <init-param>
+      <param-name>swagger.api.basepath</param-name>
+      <param-value>http://localhost:8080/api/</param-value>
+    </init-param>
+    <load-on-startup>2</load-on-startup>
+  </servlet>
+  <servlet>
+    <servlet-name>Bootstrap</servlet-name>
+    <servlet-class>org.apache.submarine.server.Bootstrap</servlet-class>
+    <load-on-startup>2</load-on-startup>
+  </servlet>
+
+  <servlet-mapping>
+    <servlet-name>default</servlet-name>
+    <url-pattern>/v2/*</url-pattern>

Review comment:
       do you mean swagger v3?




----------------------------------------------------------------
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] [submarine] tangzhankun edited a comment on pull request #271: SUBMARINE-481. Use Swagger to describe and document submarine RESTful APIs

Posted by GitBox <gi...@apache.org>.
tangzhankun edited a comment on pull request #271:
URL: https://github.com/apache/submarine/pull/271#issuecomment-620967657


   @kevin85421 thanks for the contribution. I'm wondering is it better to support the swagger API document in this PR too?


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