Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ZEPPELIN-2106] providing paragraph config in create note/paragraph rest call #2099

Closed
wants to merge 2 commits into from

Conversation

Remilito
Copy link
Contributor

@Remilito Remilito commented Mar 6, 2017

What is this PR for?

  • Allow to provide full paragraph config directly in the Create Paragraph and Create Note endpoint.
  • This saves some calls to [noteId]/paragraph/[paragraphId]/config
  • Updated doc.

What type of PR is it?

Improvement

Todos

What is the Jira issue?

ZEPPELIN-2106

How should this be tested?

Outline the steps to test the PR here.

  1. Clone the first paragraph of 'Zeppelin Tutorial/Basic Features (Spark)' to get the bank data loaded in a new note.
  2. curl -X POST -d @testAPI.json http://localhost:8080/api/notebook/$YOURNOTEID/paragraph
  3. When running the paragraphes, the graphs will show up with the appropriate settings.

testAPI.json:
{ "title":"Example providing config", "text":"%sql\nselect age, marital, count(1) cvalue from bank group by age, marital order by age", "config": { "title":true, "colWidth":6.0, "results": [ { "graph": { "mode": "scatterChart", "optionOpen": true } } ] }, "colWidth":9.0 }

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

note/paragraph call
* Allow to describe graph, colWidth, showTitle or full paragraph config
directly in the Create Paragraph and Create Note endpoint.
* Updated doc.
@Leemoonsoo
Copy link
Member

Thanks @Remilito for the contribution!

Could you elaborate why do you support two different forms of message?

    {
      ...
      "graph": {
        "mode": "pieChart"
      }
    }

and

    {
      ...
      "config": {
        ...
        "results": [
          {
            "graph": {
              "mode": "scatterChart",
              "optionOpen": true
            }
          }
        ]
      }
    }

@Remilito
Copy link
Contributor Author

@Leemoonsoo : the most common usages would be to set visual paramaters, i.e. graph options, paragraph width and paragraph title show, so the idea was to provide a more direct way to configure those rather than having to provide the whole config:{results:[{} .... to do so. Anyway, this is open to discussions

@Leemoonsoo
Copy link
Member

@Remilito Thanks for explain.

Let's say graph: {mode: {...}} is api A, and config: {results: [{graph: mode: {...}}]} is api B.

When user use api A with multiple results, it can be ambiguous that config will be applied to which particular result. I think this should be defined and documented. Also it'll be better documentation explain what is difference between api A and api B.

Alternative is just supporting api B, which i think superset of api A.

@Remilito Which do you prefer?

@Remilito
Copy link
Contributor Author

@Leemoonsoo thanks for the feedback, I think we should keep only B. I'll update the PR in a couple of days.

@Remilito
Copy link
Contributor Author

@Leemoonsoo : updated!

@Leemoonsoo
Copy link
Member

@Remilito Thanks for great work!

LGTM and merge to master if no further discussions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants