You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@livy.apache.org by GitBox <gi...@apache.org> on 2020/04/16 19:20:39 UTC
[GitHub] [incubator-livy] tmnd1991 opened a new pull request #290:
[LIVY-758] Document how to attach to an existing session from Java client
tmnd1991 opened a new pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290
## What changes were proposed in this pull request?
Add an explicit way to set the session when connecting from the Java client.
https://issues.apache.org/jira/browse/LIVY-758
## How was this patch tested?
Unit tested through `HttpClientSpec`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r409817402
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,30 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ *
+ * @param uri the uri of the livy server,
+ * if the uri contains <pre>sessions/{sessionId}</pre>
+ * the client will connect to the existing session,
+ * otherwise it will create a new session.
+ * @return the builder itself.
+ */
public LivyClientBuilder setURI(URI uri) {
config.setProperty(LIVY_URI_KEY, uri.toString());
return this;
}
+ /**
+ *
+ * @param sessionId the id of the session to attach to,
+ * if not set a new session will be created when the client is built.
+ * @return the builder itself.
+ */
+ public LivyClientBuilder setSessionId(int sessionId) {
+ config.setProperty(LIVY_SESSION_ID_KEY, "" + sessionId);
Review comment:
```suggestion
config.setProperty(LIVY_SESSION_ID_KEY, String.valueOf(sessionId));
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] tmnd1991 commented on issue #290: [LIVY-758]
Document how to attach to an existing session from Java client
Posted by GitBox <gi...@apache.org>.
tmnd1991 commented on issue #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#issuecomment-615120819
> @tmnd1991 A heads up, when you force push updates to commits it makes it difficult to track your changes as you address review (eg. I can't tell which of @mgaido91 comments you've addressed because there's no history to compare to). Since we squash all commits in a PR before merging the quantity of commits in a PR isn't an issue.
Good, I did not know if the squash before merge was a "thing" in Livy repo, I won't do the same mistake again :)
> As for your code, I read through it and I don't see any issues with the current code, but I don't have to time check it out and more thoroughly test it so I will defer to @mgaido91 on final approval for this PR.
👍
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r409816299
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,30 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ *
+ * @param uri the uri of the livy server,
+ * if the uri contains <pre>sessions/{sessionId}</pre>
+ * the client will connect to the existing session,
+ * otherwise it will create a new session.
+ * @return the builder itself.
Review comment:
```suggestion
* @return The builder itself.
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] tmnd1991 commented on issue #290: [LIVY-758]
Document how to attach to an existing session from Java client
Posted by GitBox <gi...@apache.org>.
tmnd1991 commented on issue #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#issuecomment-614848434
I could not assign myself the relate jira, how can I do it? @mgaido91 @vanzin @jerryshao
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] tmnd1991 commented on issue #290: [LIVY-758]
Document how to attach to an existing session from Java client
Posted by GitBox <gi...@apache.org>.
tmnd1991 commented on issue #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#issuecomment-614893992
I changed what you suggested @mgaido91 , thanks for the review.
Tagging @ajbozarth as he asked on Jira :)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r410463744
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,33 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ * Set the URI of the Livy server the client will connect to.
Review comment:
```suggestion
* Sets the URI of the Livy server the client will connect to.
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] tmnd1991 commented on issue #290: [LIVY-758]
Document how to attach to an existing session from Java client
Posted by GitBox <gi...@apache.org>.
tmnd1991 commented on issue #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#issuecomment-614886419
> @tmnd1991 you can't, JIRAs are assigned when related patches are merged and they are resolved
Then I think [contributing section](https://livy.apache.org/community/) is wrong :)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] tmnd1991 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
tmnd1991 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r409840092
##########
File path: client-http/src/main/java/org/apache/livy/client/http/HttpClient.java
##########
@@ -54,17 +55,18 @@
this.config = httpConf;
this.stopped = false;
- // If the given URI looks like it refers to an existing session, then try to connect to
- // an existing session. Note this means that any Spark configuration in httpConf will be
- // unused.
- Matcher m = Pattern.compile("(.*)" + LivyConnection.SESSIONS_URI + "/([0-9]+)")
- .matcher(uri.getPath());
-
+ Matcher m = Pattern.compile("(.*)" + LivyConnection.SESSIONS_URI + "/([0-9]+)").matcher(uri.getPath());
Review comment:
I did it because the in-line comment was the *only* doc about connecting to an existing session. Now that is documented in the builder I don't think the inline comment is still needed, anyway no problem at all at reverting that change.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r409814194
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,30 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ *
Review comment:
If you are adding the doc here, you should provide a description of the method...
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on issue #290: [LIVY-758]
Document how to attach to an existing session from Java client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on issue #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#issuecomment-614863859
@tmnd1991 you can't, JIRAs are assigned when related patches are merged and they are resolved
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r410463803
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,33 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ * Set the URI of the Livy server the client will connect to.
+ * If the URI contains <pre>sessions/{sessionId}</pre>,
+ * the client will connect to the specified existing session,
Review comment:
```suggestion
* the client will connect to the specified existing session;
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r409815944
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,30 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ *
+ * @param uri the uri of the livy server,
+ * if the uri contains <pre>sessions/{sessionId}</pre>
+ * the client will connect to the existing session,
+ * otherwise it will create a new session.
Review comment:
```suggestion
* @param uri The URI of Livy server. If the URI contains <pre>sessions/{sessionId}</pre>,
* the client will connect to the specified existing session, otherwise it will create a
* new session.
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r410464914
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,33 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ * Set the URI of the Livy server the client will connect to.
+ * If the URI contains <pre>sessions/{sessionId}</pre>,
+ * the client will connect to the specified existing session,
+ * otherwise it will create a new session.
+ *
+ * @param uri The URI of Livy server.
+ * @return The builder itself.
+ */
public LivyClientBuilder setURI(URI uri) {
config.setProperty(LIVY_URI_KEY, uri.toString());
return this;
}
+ /**
+ * Sets the sessionId the client will connect to. If sessionId is set,
+ * all Spark configurations will be ignored and the original session ones will be used.
Review comment:
```suggestion
* the chosen session will be used with its own configurations, so Spark configurations will be ignored.
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r409818543
##########
File path: client-http/src/main/java/org/apache/livy/client/http/HttpClient.java
##########
@@ -54,17 +55,18 @@
this.config = httpConf;
this.stopped = false;
- // If the given URI looks like it refers to an existing session, then try to connect to
- // an existing session. Note this means that any Spark configuration in httpConf will be
- // unused.
- Matcher m = Pattern.compile("(.*)" + LivyConnection.SESSIONS_URI + "/([0-9]+)")
- .matcher(uri.getPath());
-
+ Matcher m = Pattern.compile("(.*)" + LivyConnection.SESSIONS_URI + "/([0-9]+)").matcher(uri.getPath());
+ String sessionIdFromConf = httpConf.get(LivyClientBuilder.LIVY_SESSION_ID_KEY);
try {
- if (m.matches()) {
+ if (sessionIdFromConf != null && m.matches()) {
+ throw new IllegalArgumentException("Cannot set existing session both from URI and configuration");
+ } else if (sessionIdFromConf != null) {
+ this.conn = new LivyConnection(uri, httpConf);
+ this.sessionId = Integer.parseInt(sessionIdFromConf);
+ conn.post(null, SessionInfo.class, "/%d/connect", sessionId);
+ } else if (m.matches()) {
URI base = new URI(uri.getScheme(), uri.getUserInfo(), uri.getHost(), uri.getPort(),
- m.group(1), uri.getQuery(), uri.getFragment());
-
+ m.group(1), uri.getQuery(), uri.getFragment());
Review comment:
please revert unneeded change
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] ajbozarth commented on issue #290: [LIVY-758]
Document how to attach to an existing session from Java client
Posted by GitBox <gi...@apache.org>.
ajbozarth commented on issue #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#issuecomment-614941488
@mgaido91 I think your mixing up how Spark handles JIRA assignments with Livy's policy. We assign JIRAs once a PR is open for the issue. For some reason though, I can't assign issues to @tmnd1991 in the Livy JIRA at the moment. We had this issue before but I can't remember how we solved it.
@tmnd1991 A heads up, when you force push updates to commits it makes it difficult to track your changes as you address review (eg. I can't tell which of @mgaido91 comments you've addressed because there's no history to compare to). Since we squash all commits in a PR before merging the quantity of commits in a PR isn't an issue.
As for your code, I read through it and I don't see any issues with the current code, but I don't have to time check it out and more thoroughly test it so I will defer to @mgaido91 on final approval for this PR.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r410464142
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,33 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ * Set the URI of the Livy server the client will connect to.
+ * If the URI contains <pre>sessions/{sessionId}</pre>,
+ * the client will connect to the specified existing session,
+ * otherwise it will create a new session.
+ *
+ * @param uri The URI of Livy server.
+ * @return The builder itself.
+ */
public LivyClientBuilder setURI(URI uri) {
config.setProperty(LIVY_URI_KEY, uri.toString());
return this;
}
+ /**
+ * Sets the sessionId the client will connect to. If sessionId is set,
Review comment:
```suggestion
* Sets the session ID the client will connect to. If a session ID is set,
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r409818115
##########
File path: client-http/src/main/java/org/apache/livy/client/http/HttpClient.java
##########
@@ -54,17 +55,18 @@
this.config = httpConf;
this.stopped = false;
- // If the given URI looks like it refers to an existing session, then try to connect to
- // an existing session. Note this means that any Spark configuration in httpConf will be
- // unused.
- Matcher m = Pattern.compile("(.*)" + LivyConnection.SESSIONS_URI + "/([0-9]+)")
- .matcher(uri.getPath());
-
+ Matcher m = Pattern.compile("(.*)" + LivyConnection.SESSIONS_URI + "/([0-9]+)").matcher(uri.getPath());
Review comment:
why did you remove the comment above? moreover, here I see no changes, so please revert the change to keep the history clean
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r409813168
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -36,9 +36,10 @@
public final class LivyClientBuilder {
public static final String LIVY_URI_KEY = "livy.uri";
+ public static final String LIVY_SESSION_ID_KEY = "livy.sessionId";
private static final ServiceLoader<LivyClientFactory> CLIENT_FACTORY_LOADER =
- ServiceLoader.load(LivyClientFactory.class, classLoader());
+ ServiceLoader.load(LivyClientFactory.class, classLoader());
Review comment:
nit: unneeded change, please revert
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on issue #290: [LIVY-758]
Document how to attach to an existing session from Java client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on issue #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#issuecomment-615457943
> @mgaido91 I think your mixing up how Spark handles JIRA assignments with Livy's policy. We assign JIRAs once a PR is open for the issue. For some reason though, I can't assign issues to @tmnd1991 in the Livy JIRA at the moment. We had this issue before but I can't remember how we solved it.
Sorry, I saw the same pattern being followed in Livy too in practice so I assumed it was the same. I think the point is that @tmnd1991 has to be added to the contributors in JIRA. I'll do that when resolving the JIRA.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r409816381
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,30 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ *
+ * @param uri the uri of the livy server,
+ * if the uri contains <pre>sessions/{sessionId}</pre>
+ * the client will connect to the existing session,
+ * otherwise it will create a new session.
+ * @return the builder itself.
+ */
public LivyClientBuilder setURI(URI uri) {
config.setProperty(LIVY_URI_KEY, uri.toString());
return this;
}
+ /**
+ *
Review comment:
ditto
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request
#290: [LIVY-758] Document how to attach to an existing session from Java
client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r409816963
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,30 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ *
+ * @param uri the uri of the livy server,
+ * if the uri contains <pre>sessions/{sessionId}</pre>
+ * the client will connect to the existing session,
+ * otherwise it will create a new session.
+ * @return the builder itself.
+ */
public LivyClientBuilder setURI(URI uri) {
config.setProperty(LIVY_URI_KEY, uri.toString());
return this;
}
+ /**
+ *
+ * @param sessionId the id of the session to attach to,
+ * if not set a new session will be created when the client is built.
Review comment:
```suggestion
* @param sessionId The ID of the session to attach to. If not set, a new
* session will be created when the client is built.
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-livy] mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290:
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r411248523
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,32 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ * Sets the URI of the Livy server the client will connect to. If the URI contains
+ * <pre>sessions/{sessionId}</pre>, the client will connect to the specified existing session;
Review comment:
yes, we definitely need to leave it for now, but we can start deprecating it. Let's wait for others opinion .
----------------------------------------------------------------
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] [incubator-livy] mgaido91 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
Posted by GitBox <gi...@apache.org>.
mgaido91 commented on a change in pull request #290:
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r411242416
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,32 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ * Sets the URI of the Livy server the client will connect to. If the URI contains
+ * <pre>sessions/{sessionId}</pre>, the client will connect to the specified existing session;
Review comment:
one last question: @ajbozarth @jerryshao @tmnd1991 I think that by adding this API, we might want to deprecate this "hacky" approach and eventually remove the support to it in next major release. Any thought on this?
----------------------------------------------------------------
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] [incubator-livy] tmnd1991 commented on a change in pull request #290: [LIVY-758] Document how to attach to an existing session from Java client
Posted by GitBox <gi...@apache.org>.
tmnd1991 commented on a change in pull request #290:
URL: https://github.com/apache/incubator-livy/pull/290#discussion_r411246522
##########
File path: api/src/main/java/org/apache/livy/LivyClientBuilder.java
##########
@@ -96,11 +97,32 @@ public LivyClientBuilder(boolean loadDefaults) throws IOException {
}
}
+ /**
+ * Sets the URI of the Livy server the client will connect to. If the URI contains
+ * <pre>sessions/{sessionId}</pre>, the client will connect to the specified existing session;
Review comment:
I 100% agree on removing the hacky (and until now not even documented) approach. I left it to make the PR backward compatible
----------------------------------------------------------------
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