Skip to content

Unbind MetastoreTypeConfig in Hive tests#14059

Merged
findepi merged 2 commits intotrinodb:masterfrom
findepi:findepi/unbind-metastoretypeconfig-in-hive-tests-ea12bb
Sep 9, 2022
Merged

Unbind MetastoreTypeConfig in Hive tests#14059
findepi merged 2 commits intotrinodb:masterfrom
findepi:findepi/unbind-metastoretypeconfig-in-hive-tests-ea12bb

Conversation

@findepi
Copy link
Copy Markdown
Member

@findepi findepi commented Sep 8, 2022

This removes MetastoreTypeConfig binding added in
findepi@7229738 (#13707) and replaces it with an
annotated boolean, decoupling the HiveMetadata logic from metastore
implementations.

@cla-bot cla-bot bot added the cla-signed label Sep 8, 2022
@findepi findepi force-pushed the findepi/unbind-metastoretypeconfig-in-hive-tests-ea12bb branch from e6d860b to 3ff201a Compare September 8, 2022 13:08
@findepi findepi marked this pull request as ready for review September 8, 2022 13:08
@findepi findepi requested review from alexjo2144 and homar September 8, 2022 13:09
@homar
Copy link
Copy Markdown
Member

homar commented Sep 8, 2022

I am ashamed I haven't done it in the first place.

@homar
Copy link
Copy Markdown
Member

homar commented Sep 8, 2022

Just a thought if you did it for hive, maybe it would make sense to do it the same way for DeltaLake as well ?

Copy link
Copy Markdown
Contributor

@ksobolew ksobolew left a comment

Choose a reason for hiding this comment

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

Thanks! (Sorry if I communicated the problem with this binding in a not great way)

@findepi findepi force-pushed the findepi/unbind-metastoretypeconfig-in-hive-tests-ea12bb branch from 3ff201a to a0d2e9c Compare September 8, 2022 15:46
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Sep 8, 2022

Just a thought if you did it for hive, maybe it would make sense to do it the same way for DeltaLake as well ?

@homar do you have a pointer to the code you'd want to see changed?
or maybe you want to do this? :)

@homar
Copy link
Copy Markdown
Member

homar commented Sep 8, 2022

Just a thought if you did it for hive, maybe it would make sense to do it the same way for DeltaLake as well ?

@homar do you have a pointer to the code you'd want to see changed? or maybe you want to do this? :)

I will try to do it

@homar homar closed this Sep 8, 2022
@homar homar reopened this Sep 8, 2022
@homar
Copy link
Copy Markdown
Member

homar commented Sep 8, 2022

sorry I missclicked, I didn't mean to close the PR ;/

@homar
Copy link
Copy Markdown
Member

homar commented Sep 8, 2022

Just a thought if you did it for hive, maybe it would make sense to do it the same way for DeltaLake as well ?

@homar do you have a pointer to the code you'd want to see changed? or maybe you want to do this? :)

#14066

@findepi findepi force-pushed the findepi/unbind-metastoretypeconfig-in-hive-tests-ea12bb branch from a0d2e9c to 5ab2970 Compare September 9, 2022 07:04
This removes `MetastoreTypeConfig` binding added in
7229738 and replaces it with an
annotated boolean, decoupling the `HiveMetadata` logic from metastore
implementations.
@findepi findepi force-pushed the findepi/unbind-metastoretypeconfig-in-hive-tests-ea12bb branch from 5ab2970 to f68dbbf Compare September 9, 2022 08:09
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Sep 9, 2022

CI #13889

@findepi findepi merged commit c946407 into trinodb:master Sep 9, 2022
@findepi findepi deleted the findepi/unbind-metastoretypeconfig-in-hive-tests-ea12bb branch September 9, 2022 18:05
@github-actions github-actions bot added this to the 396 milestone Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants