You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/09/28 15:17:56 UTC
[GitHub] [incubator-kyuubi] turboFei opened a new pull request, #3577: transfer connection url
turboFei opened a new pull request, #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/CONTRIBUTING.html
2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei closed pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei closed pull request #3577: Transfer connection url when opening session
URL: https://github.com/apache/incubator-kyuubi/pull/3577
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#discussion_r984566944
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/session/Session.scala:
##########
@@ -33,6 +33,7 @@ trait Session {
def user: String
def password: String
def ipAddress: String
+ def connectionUrl: String
Review Comment:
just for `KyuubiSession`?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#issuecomment-1261371512
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3577](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (768646b) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/8419b7bafd0e39115f9b11b29e5b6f3c7f89451d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8419b7b) will **increase** coverage by `0.02%`.
> The diff coverage is `95.23%`.
```diff
@@ Coverage Diff @@
## master #3577 +/- ##
============================================
+ Coverage 51.74% 51.77% +0.02%
Complexity 13 13
============================================
Files 482 482
Lines 26975 27001 +26
Branches 3762 3762
============================================
+ Hits 13959 13980 +21
- Misses 11661 11665 +4
- Partials 1355 1356 +1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ache/kyuubi/server/KyuubiRestFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvS3l1dWJpUmVzdEZyb250ZW5kU2VydmljZS5zY2FsYQ==) | `79.74% <ø> (-1.21%)` | :arrow_down: |
| [...g/apache/kyuubi/client/api/v1/dto/SessionData.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXJlc3QtY2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9reXV1YmkvY2xpZW50L2FwaS92MS9kdG8vU2Vzc2lvbkRhdGEuamF2YQ==) | `41.86% <50.00%> (+0.83%)` | :arrow_up: |
| [.../engine/spark/session/SparkSQLSessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9zZXNzaW9uL1NwYXJrU1FMU2Vzc2lvbk1hbmFnZXIuc2NhbGE=) | `80.00% <100.00%> (+0.25%)` | :arrow_up: |
| [...kyuubi/engine/spark/session/SparkSessionImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9zZXNzaW9uL1NwYXJrU2Vzc2lvbkltcGwuc2NhbGE=) | `75.60% <100.00%> (+5.02%)` | :arrow_up: |
| [...apache/kyuubi/service/AbstractBackendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL0Fic3RyYWN0QmFja2VuZFNlcnZpY2Uuc2NhbGE=) | `98.36% <100.00%> (ø)` | |
| [...a/org/apache/kyuubi/service/TFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `91.20% <100.00%> (-0.27%)` | :arrow_down: |
| [...la/org/apache/kyuubi/session/AbstractSession.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0Fic3RyYWN0U2Vzc2lvbi5zY2FsYQ==) | `96.33% <100.00%> (+0.03%)` | :arrow_up: |
| [...ala/org/apache/kyuubi/session/SessionManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL1Nlc3Npb25NYW5hZ2VyLnNjYWxh) | `86.01% <100.00%> (ø)` | |
| [.../org/apache/kyuubi/events/KyuubiSessionEvent.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9ldmVudHMvS3l1dWJpU2Vzc2lvbkV2ZW50LnNjYWxh) | `96.55% <100.00%> (+3.00%)` | :arrow_up: |
| [...rg/apache/kyuubi/server/BackendServiceMetric.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvQmFja2VuZFNlcnZpY2VNZXRyaWMuc2NhbGE=) | `93.44% <100.00%> (ø)` | |
| ... and [18 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/3577/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#discussion_r1012668557
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala:
##########
@@ -98,7 +98,7 @@ private[v1] class BatchesResource extends ApiRequestContext with Logging {
appUrl,
appState,
appDiagnostic,
- fe.connectionUrl,
+ session.connectionUrl,
Review Comment:
no, the connect url is frontend service connect url without config, just host and port.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#issuecomment-1261737238
Instead of continuously changing these func definitions, I would expect all newly add information to propagate via configurations
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#discussion_r1012695843
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/service/TFrontendService.scala:
##########
@@ -159,8 +159,10 @@ abstract class TFrontendService(name: String)
res.setServerProtocolVersion(protocol)
val userName = getUserName(req)
val ipAddress = getIpAddress
- val configuration = Map(KYUUBI_CLIENT_IP_KEY -> ipAddress) ++
- Option(req.getConfiguration).map(_.asScala.toMap).getOrElse(Map.empty[String, String])
+ val configuration =
+ Map(KYUUBI_CLIENT_IP_KEY -> ipAddress) ++
+ Option(req.getConfiguration).map(_.asScala.toMap).getOrElse(Map.empty[String, String]) ++
+ Map(KYUUBI_SESSION_CONNECTION_URL_KEY -> connectionUrl)
Review Comment:
re-order the KYUUBI_SESSION_CONNECTION_URL_KEY to prevent it be overwritten by client.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#issuecomment-1263055785
good idea
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#issuecomment-1301949263
hi @lightning-L could you rebase your pr 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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#discussion_r984296643
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/events/KyuubiSessionEvent.scala:
##########
@@ -61,16 +60,14 @@ case class KyuubiSessionEvent(
object KyuubiSessionEvent {
def apply(session: KyuubiSession): KyuubiSessionEvent = {
- assert(KyuubiServer.kyuubiServer != null)
- val serverIP = KyuubiServer.kyuubiServer.frontendServices.head.connectionUrl
KyuubiSessionEvent(
session.handle.identifier.toString,
session.protocol.getValue,
session.sessionType.toString,
session.name.getOrElse(""),
session.user,
session.ipAddress,
- serverIP,
+ session.connectionUrl,
Review Comment:
can we keep it in conf map?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#discussion_r1009381268
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/session/Session.scala:
##########
@@ -33,6 +33,7 @@ trait Session {
def user: String
def password: String
def ipAddress: String
+ def connectionUrl: String
Review Comment:
removed
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#issuecomment-1301795383
cc @pan3793 @ulysses-you
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#discussion_r1012670884
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala:
##########
@@ -98,7 +98,7 @@ private[v1] class BatchesResource extends ApiRequestContext with Logging {
appUrl,
appState,
appDiagnostic,
- fe.connectionUrl,
+ session.connectionUrl,
Review Comment:
before, we rely on the frontend service to get connection url.
But for the KyuubiSessionImpl, we do not know the accurate related frontend service, it might be from MysqlFrontEnd, ThriftBinary, ThriftHttp and RestFrontEnd.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on a diff in pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#discussion_r984296123
##########
kyuubi-common/src/main/scala/org/apache/kyuubi/session/Session.scala:
##########
@@ -33,6 +33,7 @@ trait Session {
def user: String
def password: String
def ipAddress: String
+ def connectionUrl: String
Review Comment:
can we remove 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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #3577: Transfer server url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#issuecomment-1261083128
how do you think about this? @yaooqinn
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#discussion_r1009381858
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/events/KyuubiSessionEvent.scala:
##########
@@ -61,16 +60,14 @@ case class KyuubiSessionEvent(
object KyuubiSessionEvent {
def apply(session: KyuubiSession): KyuubiSessionEvent = {
- assert(KyuubiServer.kyuubiServer != null)
- val serverIP = KyuubiServer.kyuubiServer.frontendServices.head.connectionUrl
KyuubiSessionEvent(
session.handle.identifier.toString,
session.protocol.getValue,
session.sessionType.toString,
session.name.getOrElse(""),
session.user,
session.ipAddress,
- serverIP,
+ session.connectionUrl,
Review Comment:
we shall reserve it to keep compatibility
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#issuecomment-1301948892
thanks @ulysses-you @yaooqinn , merged to master
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#issuecomment-1263073041
thanks, addressed.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#discussion_r984566669
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/events/KyuubiSessionEvent.scala:
##########
@@ -61,16 +60,14 @@ case class KyuubiSessionEvent(
object KyuubiSessionEvent {
def apply(session: KyuubiSession): KyuubiSessionEvent = {
- assert(KyuubiServer.kyuubiServer != null)
- val serverIP = KyuubiServer.kyuubiServer.frontendServices.head.connectionUrl
KyuubiSessionEvent(
session.handle.identifier.toString,
session.protocol.getValue,
session.sessionType.toString,
session.name.getOrElse(""),
session.user,
session.ipAddress,
- serverIP,
+ session.connectionUrl,
Review Comment:
should we remove `serverIp` field from KyuubiSessionEvent?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#discussion_r1012646142
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala:
##########
@@ -98,7 +98,7 @@ private[v1] class BatchesResource extends ApiRequestContext with Logging {
appUrl,
appState,
appDiagnostic,
- fe.connectionUrl,
+ session.connectionUrl,
Review Comment:
the only difference is, session connection url contains some configs ?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] pan3793 commented on pull request #3577: Transfer connection url when opening session
Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #3577:
URL: https://github.com/apache/incubator-kyuubi/pull/3577#issuecomment-1303462717
Seems CI hangs starting from this PR. @turboFei @lightning-L would you please take a look?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org