Skip to content

Fetching databricks host from connection if not supplied in extras.#10762

Merged
XD-DENG merged 2 commits intoapache:masterfrom
joshi95:databricks-hook-host-fix
Sep 18, 2020
Merged

Fetching databricks host from connection if not supplied in extras.#10762
XD-DENG merged 2 commits intoapache:masterfrom
joshi95:databricks-hook-host-fix

Conversation

@joshi95
Copy link
Contributor

@joshi95 joshi95 commented Sep 6, 2020

Fix for the issue #10726 . Have supported the previous functionality of fetching the host from extras if specified else it is getting fetched from the databricks connection which makes more sense.

@XD-DENG
Copy link
Member

XD-DENG commented Sep 18, 2020

Hi @joshi95 , looks good to me. May you rebase?

@XD-DENG
Copy link
Member

XD-DENG commented Sep 18, 2020

On the other hand, before I can merge, the test you added doesn't make much sense to me. May you clarify a bit or enhance it further?

@joshi95
Copy link
Contributor Author

joshi95 commented Sep 18, 2020

On the other hand, before I can merge, the test you added doesn't make much sense to me. May you clarify a bit or enhance it further?

Hi, In this test I have reused the existing TestDatabricksHookToken testcase with one less extra argument ie {'token': TOKEN} instead of {'token': TOKEN, 'host': HOST}

@joshi95 joshi95 force-pushed the databricks-hook-host-fix branch from 15cafc7 to 384f2cf Compare September 18, 2020 08:40
@XD-DENG
Copy link
Member

XD-DENG commented Sep 18, 2020

I see. Thanks for the clarification.

May you rebase? Once ci is green I would be happy to merge.

Thanks.

@joshi95
Copy link
Contributor Author

joshi95 commented Sep 18, 2020

I see. Thanks for the clarification.

May you rebase? Once ci is green I would be happy to merge.

Thanks.

Yes thanks, Have rebased .

@joshi95
Copy link
Contributor Author

joshi95 commented Sep 18, 2020

Hi not sure why k8s ci test has failed.

@XD-DENG
Copy link
Member

XD-DENG commented Sep 18, 2020

Possibly a transient error. I have restarted the CI, let’s see if it will be ok

@joshi95
Copy link
Contributor Author

joshi95 commented Sep 18, 2020

Possibly a transient error. I have restarted the CI, let’s see if it will be ok

thanks

@XD-DENG XD-DENG merged commit 966a06d into apache:master Sep 18, 2020
@XD-DENG
Copy link
Member

XD-DENG commented Sep 18, 2020

Thanks @joshi95 for the contribution 😃

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.

databricks host name pulling out from extra json data when token is used

3 participants