----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57582/#review169045 -----------------------------------------------------------
Ship it! Overall, the patch looks fine to me, just a few comments: 1. I think maybe it would be good to eventually have more unit tests for this change. 2. Could you please add Oliver Szabo to this review? He's implemented Swagger support for the LogSearch Portal UI, and so will likely have a lot of useful feedback based on his experiences so far with Swagger and the Swagger APIs. Thanks. - Robert Nettleton On March 14, 2017, 11:35 p.m., Jaimin Jetly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57582/ > ----------------------------------------------------------- > > (Updated March 14, 2017, 11:35 p.m.) > > > Review request for Ambari, Jonathan Hurley, Jayush Luniya, Nate Cole, Robert > Nettleton, Sumit Mohanty, Sid Wagle, and Yusaku Sako. > > > Bugs: AMBARI-20436 > https://issues.apache.org/jira/browse/AMBARI-20436 > > > Repository: ambari > > > Description > ------- > > A separate branch is created for this work: ambari-rest-api-explorer > > ## As part of this task, following changes are done: > 1. Users, Groups and Views API are integrated with swagger and exposed from > ambari rest api explorer ui (swagger ui) on a deployed cluster at path: > http://c6404.ambari.apache.org:8080/api-docs > 2. swagger-maven-plugin is used to generate swagger.json file on compile > time. This file is published in web resources directory. Note that this file > is generated build time and will be available on deployed ambari-server host > at web resources location but it is not yet decided to be committed and > maintained in Ambari source code > 3. swagger2markup-maven-plugin is used to generate asciidoc from swagger.json > file (that can be shown as markdown in github). More information about this > format an be found at http://asciidoc.org/ and http://asciidoctor.org/. This > generates files in docs/api/asciidoc/** location at build time. This > directory is currently intended to be committed and maintained in ambari > source code > 4. swagger-ui (version: v2.1.1-M2) compiled code with the different css skin > (adopted from [link|https://github.com/jensoleg/swagger-ui]) is committed to > ambari-web/api-docs directory with certain modification to make it work with > ambari api. Further ui polishing will be done in subsequent tasks. Also there > is a strong possibility to maintain the fork code of swagger-ui and compile > (minify and concanate) it during ambari compile time rather than directly > using swagger-ui dist files. Doing so will help developers when customization > done over swagger-ui will increase > 5. swagger-annotation expects application to define schema of request body > and response for each endpoint to be encapsulated in a class. While Ambari > follows this pattern for some of the endpoint, there are many others which > does not do so. For The ones which do not does so, new request and response > classes were defined. Going forward at the completion of this epic, either > each resource type or each resource provider should be coupled with a > resource response class and a resource request class. As part of this patch, > each resourceprovider worked upon introduces a new method "getResponse". At > completion of this epic ResourceProvider interface should also declare > methods like "getResponse" and "getRequest" that returns response schema and > request schema instances for the resource API endpoints > 6. Currently it seems that swagger has a limitation in supporting > "subresource locator methods". This issue is been reported to swagger > community and is being tracked at > [link|https://github.com/swagger-api/swagger-core/issues/2136]. As a result > of which currently as a temporary workaround, all subresources are converted > to root resources. Also all root resources on similar path are moved under > same subpackages (like userApi, groupApi and viewApi). > > > Diffs > ----- > > LICENSE.txt f05016f > ambari-server/docs/api/asciidoc/definitions.adoc PRE-CREATION > ambari-server/docs/api/asciidoc/overview.adoc PRE-CREATION > ambari-server/docs/api/asciidoc/paths.adoc PRE-CREATION > ambari-server/pom.xml f0c73e4 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ActiveWidgetLayoutService.java > 40cd6e0 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/GroupPrivilegeService.java > 9c1f1a3 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/GroupService.java > 8ec0097 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/MemberService.java > 28e53e6 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/UserAuthorizationService.java > 26a7107 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/UserPrivilegeService.java > f9c95e7 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/UserService.java > 31f3a8c > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewDataMigrationService.java > 2a9aa64 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewExternalSubResourceService.java > 66ccae7 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewInstanceService.java > 53d4918 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewPermissionService.java > 8f7f4ef > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewPrivilegeService.java > 7393745 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewService.java > 17a9f34 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewSubResourceService.java > 76d28fe > > ambari-server/src/main/java/org/apache/ambari/server/api/services/ViewVersionService.java > 3554da1 > > ambari-server/src/main/java/org/apache/ambari/server/api/services/groupApi/GroupPrivilegeService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/services/userApi/ActiveWidgetLayoutService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/services/userApi/UserAuthorizationService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/services/userApi/UserPrivilegeService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/services/viewApi/ViewPermissionService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/api/services/viewApi/ViewPrivilegeService.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ActiveWidgetLayoutRequest.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ActiveWidgetLayoutResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ApiModel.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/GroupPrivilegeResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/GroupRequest.java > 1bc18cc > > ambari-server/src/main/java/org/apache/ambari/server/controller/GroupResponse.java > 0baccc7 > > ambari-server/src/main/java/org/apache/ambari/server/controller/MemberRequest.java > 0245f36 > > ambari-server/src/main/java/org/apache/ambari/server/controller/MemberResponse.java > 3dc6558 > > ambari-server/src/main/java/org/apache/ambari/server/controller/PrivilegeResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/UserAuthorizationResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/UserPrivilegeResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/UserRequest.java > 282131a > > ambari-server/src/main/java/org/apache/ambari/server/controller/UserResponse.java > b90f864 > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewInstanceRequest.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewInstanceResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewPermissionResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewPrivilegeRequest.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewPrivilegeResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/ViewVersionResponse.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java > d9a7997 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/GroupPrivilegeResourceProvider.java > bf63794 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserAuthorizationResourceProvider.java > 8193a49 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserPrivilegeResourceProvider.java > b9b756b > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewPermissionResourceProvider.java > 4f0a6f0 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewVersionResourceProvider.java > 4055f1a > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PrincipalTypeEntity.java > e95223a > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/User.java > 18509d3 > ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java > 81c4734 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/ActiveWidgetLayoutServiceTest.java > adae956 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/GroupPrivilegeServiceTest.java > 995dfaf > > ambari-server/src/test/java/org/apache/ambari/server/api/services/GroupServiceTest.java > b171ceb > > ambari-server/src/test/java/org/apache/ambari/server/api/services/MemberServiceTest.java > 1e0137e > > ambari-server/src/test/java/org/apache/ambari/server/api/services/UserAuthorizationServiceTest.java > 8a2f799 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/UserPrivilegeServiceTest.java > 7b0e06d > > ambari-server/src/test/java/org/apache/ambari/server/api/services/ViewDataMigrationServiceTest.java > e1eb0de > > ambari-server/src/test/java/org/apache/ambari/server/api/services/ViewExternalSubResourceServiceTest.java > 155f91e > > ambari-server/src/test/java/org/apache/ambari/server/api/services/ViewPermissionServiceTest.java > 7f58fb2 > > ambari-server/src/test/java/org/apache/ambari/server/api/services/ViewSubResourceServiceTest.java > 9499466 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/GroupPrivilegeResourceProviderTest.java > ebe92e4 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UserPrivilegeResourceProviderTest.java > 5092db5 > ambari-web/api-docs/css/api-explorer.css PRE-CREATION > ambari-web/api-docs/css/index.css PRE-CREATION > ambari-web/api-docs/css/print.css PRE-CREATION > ambari-web/api-docs/css/reset.css PRE-CREATION > ambari-web/api-docs/css/screen.css PRE-CREATION > ambari-web/api-docs/css/standalone.css PRE-CREATION > ambari-web/api-docs/css/typography.css PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-700.eot PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-700.svg PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-700.ttf PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-700.woff PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-700.woff2 PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-regular.eot PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-regular.svg PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-regular.ttf PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-regular.woff PRE-CREATION > ambari-web/api-docs/fonts/droid-sans-v6-latin-regular.woff2 PRE-CREATION > ambari-web/api-docs/images/Swagger_explorer.png PRE-CREATION > ambari-web/api-docs/images/Swagger_explorer_min.png PRE-CREATION > ambari-web/api-docs/images/explorer_icons.png PRE-CREATION > ambari-web/api-docs/images/favicon-16x16.png PRE-CREATION > ambari-web/api-docs/images/favicon-32x32.png PRE-CREATION > ambari-web/api-docs/images/favicon.ico PRE-CREATION > ambari-web/api-docs/images/json_editor_integration.png PRE-CREATION > ambari-web/api-docs/images/logo_small.png PRE-CREATION > ambari-web/api-docs/images/pet_store_api.png PRE-CREATION > ambari-web/api-docs/images/senodio.png PRE-CREATION > ambari-web/api-docs/images/throbber.gif PRE-CREATION > ambari-web/api-docs/images/wordnik_api.png PRE-CREATION > ambari-web/api-docs/index.html PRE-CREATION > ambari-web/api-docs/lib/backbone-min.js PRE-CREATION > ambari-web/api-docs/lib/bootstrap.min.js PRE-CREATION > ambari-web/api-docs/lib/handlebars-2.0.0.js PRE-CREATION > ambari-web/api-docs/lib/highlight.7.3.pack.js PRE-CREATION > ambari-web/api-docs/lib/jquery-1.8.0.min.js PRE-CREATION > ambari-web/api-docs/lib/jquery.ba-bbq.min.js PRE-CREATION > ambari-web/api-docs/lib/jquery.slideto.min.js PRE-CREATION > ambari-web/api-docs/lib/jquery.wiggle.min.js PRE-CREATION > ambari-web/api-docs/lib/jsoneditor.js PRE-CREATION > ambari-web/api-docs/lib/marked.js PRE-CREATION > ambari-web/api-docs/lib/swagger-oauth.js PRE-CREATION > ambari-web/api-docs/lib/underscore-min.js PRE-CREATION > ambari-web/api-docs/lib/underscore-min.map PRE-CREATION > ambari-web/api-docs/o2c.html PRE-CREATION > ambari-web/api-docs/swagger-ui.js PRE-CREATION > ambari-web/api-docs/swagger-ui.min.js PRE-CREATION > ambari-web/brunch-config.js 31eb1cb > > > Diff: https://reviews.apache.org/r/57582/diff/2/ > > > Testing > ------- > > Tested on dev environment for all integrated apis to be working from ui > interface > Verified that Rat check, style check and existing ambari-server unit tests > are passing with the patch > > > Thanks, > > Jaimin Jetly > >
