Skip to content

Conversation

@zjffdu
Copy link
Contributor

@zjffdu zjffdu commented Sep 26, 2017

What is this PR for?

This PR is trying to allow user to add custom http headers when calling livy rest api. User just need to specify zeppelin.livy.http.headers in livy interpreter setting

What type of PR is it?

[Feature]

Todos

  • - Task

What is the Jira issue?

How should this be tested?

Outline the steps to test the PR here.

Screenshots (if appropriate)

Questions:

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

</tr>
<tr>
<td>zeppelin.livy.http.headers</td>
<td>key_1: value_1; key_2: value_2</td>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are ; and : valid separators that would not be present in any header itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a concrete example of what this could be useful for?

@zjffdu
Copy link
Contributor Author

zjffdu commented Sep 28, 2017

Custom http headers is needed when integrating knox with livy interpreter.

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one comment

String[] headers = property.getProperty("zeppelin.livy.http.headers").split(";");
for (String header : headers) {
String[] splits = header.split(":");
customHeaders.put(splits[0].trim(), splits[1].trim());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handle the case when splits[1] is empty?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per rfc, looks like : is required but field value is not.
we should probably error out if : is missing but handle empty value

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@zjffdu zjffdu force-pushed the ZEPPELIN-2953 branch 2 times, most recently from 301f8bb to a615f00 Compare October 12, 2017 06:52
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.

3 participants