Skip to content

ZEPPELIN-238: Remove unused swagger code#226

Closed
rohitagarwal003 wants to merge 1 commit into
apache:masterfrom
rohitagarwal003:ZEPPELIN-238
Closed

ZEPPELIN-238: Remove unused swagger code#226
rohitagarwal003 wants to merge 1 commit into
apache:masterfrom
rohitagarwal003:ZEPPELIN-238

Conversation

@rohitagarwal003
Copy link
Copy Markdown
Contributor

No description provided.

@Leemoonsoo
Copy link
Copy Markdown
Member

Looks good to me!

@nberserk
Copy link
Copy Markdown
Contributor

no use of swagger in the future also ?

@rohitagarwal003
Copy link
Copy Markdown
Contributor Author

We can add swagger back if and when it is required. Right now, this was just dead code.

@eranwitkon
Copy link
Copy Markdown

I think having an updated Swagger information including swagger.json for swagger UI is a good thing for zeppelin as it allows developers to integrate with zeppelin through services.

I am in the process of writing more REST tests and documenting the use of REST API (per the community request) and having Swagger working could help me a lot :-)

@rohitagarwal003
Copy link
Copy Markdown
Contributor Author

@Leemoonsoo Why was swagger removed earlier? Do we want to document the REST API using swagger now? If not we can merge this. If we do want to document using swagger, we should file a JIRA to fix swagger and make it workable.

@nberserk
Copy link
Copy Markdown
Contributor

I think we need more rest apis for enterprise support. and then swagger will be useful soon.

@Leemoonsoo
Copy link
Copy Markdown
Member

@mindprince zeppelin-docs has old, outdated, documentation. With the preparation of moving to ASF, zeppelin-docs was removed (ZEPL/zeppelin#289) while it's got some third-party dependencies. And that makes error on swagger, and swagger is disabled by ZEPL/zeppelin#311 to remove the error.

@Leemoonsoo
Copy link
Copy Markdown
Member

One question. I think document for REST api will be more useful when it is published in the website.
Can swagger help it?

@rohitagarwal003
Copy link
Copy Markdown
Contributor Author

Yeah, I think we should rather have static documentation of REST API on the website. Something like https://spark.apache.org/docs/latest/monitoring.html#rest-api
Running a swagger servlet wherever you are running Zeppelin Server seems like an overkill to me.

@rohitagarwal003
Copy link
Copy Markdown
Contributor Author

I see that we merged #256 which added REST API documentation to the website.

@eranwitkon @Leemoonsoo Do you think we can merge this now?

@eranwitkon
Copy link
Copy Markdown

I think we should merge PR #266 which touches the same files and PR #268 which adds the REST API test and the re-base this one and merge.

@rohitagarwal003
Copy link
Copy Markdown
Contributor Author

@Leemoonsoo @eranwitkon I see that the PRs which you mentioned were merged into master. So, I have rebased my changes on top of the current master. Please take a look. Thanks!

@eranwitkon
Copy link
Copy Markdown

+1 from me. merged, build and tested.

On Wed, Sep 9, 2015 at 9:11 AM Rohit Agarwal notifications@github.com
wrote:

@Leemoonsoo https://github.com/Leemoonsoo @eranwitkon
https://github.com/eranwitkon I see that the PRs which you mentioned
were merged into master. So, I have rebased my changes on top of the
current master. Please take a look. Thanks!


Reply to this email directly or view it on GitHub
#226 (comment)
.

Eran | "You don't need eyes to see, you need vision" (Faithless)

@Leemoonsoo
Copy link
Copy Markdown
Member

I'm merging if there're no more discussions

@asfgit asfgit closed this in b929b34 Sep 10, 2015
Leemoonsoo pushed a commit to Leemoonsoo/zeppelin that referenced this pull request Sep 17, 2015
Author: Rohit Agarwal <rohita@qubole.com>

Closes apache#226 from mindprince/ZEPPELIN-238 and squashes the following commits:

a2df01d [Rohit Agarwal] ZEPPELIN-238: Remove unused swagger code

(cherry picked from commit b929b34)
Signed-off-by: Lee moon soo <moon@apache.org>
lelou6666 pushed a commit to lelou6666/incubator-zeppelin that referenced this pull request Mar 25, 2016
Fix dynamic form by bringing back old code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants