You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Peter Vary via Review Board <no...@reviews.apache.org> on 2018/01/22 13:33:53 UTC
Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/
-----------------------------------------------------------
Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
Bugs: HIVE-18481
https://issues.apache.org/jira/browse/HIVE-18481
Repository: hive-git
Description
-------
Create IMetaStoreClient tests to cover the table query methods
Diffs
-----
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java PRE-CREATION
Diff: https://reviews.apache.org/r/65264/diff/1/
Testing
-------
Run the created tests
Thanks,
Peter Vary
Re: Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/#review196299
-----------------------------------------------------------
Ship it!
Ship It!
- Sahil Takiar
On Jan. 25, 2018, 1:01 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65264/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2018, 1:01 p.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18481
> https://issues.apache.org/jira/browse/HIVE-18481
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create IMetaStoreClient tests to cover the table query methods
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetExists.java PRE-CREATION
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesList.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/65264/diff/4/
>
>
> Testing
> -------
>
> Run the created tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/
-----------------------------------------------------------
(Updated Jan. 25, 2018, 1:01 p.m.)
Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
Changes
-------
Addessed Sahil's comments
Bugs: HIVE-18481
https://issues.apache.org/jira/browse/HIVE-18481
Repository: hive-git
Description
-------
Create IMetaStoreClient tests to cover the table query methods
Diffs (updated)
-----
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetExists.java PRE-CREATION
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesList.java PRE-CREATION
Diff: https://reviews.apache.org/r/65264/diff/4/
Changes: https://reviews.apache.org/r/65264/diff/3-4/
Testing
-------
Run the created tests
Thanks,
Peter Vary
Re: Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
> On Jan. 24, 2018, 5:50 p.m., Sahil Takiar wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
> > Lines 63 (patched)
> > <https://reviews.apache.org/r/65264/diff/3/?file=1944443#file1944443line63>
> >
> > A lot of the setup and teardown code for all these classes looks pretty similar. Is it worth creating an abstract class (something like `AbstractMetaStoreClientTest`) to encapsulate all the common setup / teardown logic?
> >
> > We don't have to do this for all the JIRAs in HIVE-18371, since most of them are already done. But would be good for future tests.
>
> Peter Vary wrote:
> When we have a junit release with this commit:
> https://github.com/junit-team/junit4/commit/1bf8438b65858565dbb64736bfe13aae9cfc1b5a
>
> Then we can change the code to something like this:
> ```
> @Parameterized.Parameters(name = "{0}")
> public static List<Object[]> getMetaStoreToTest() throws Exception {
> return MetaStoreFactoryForTests.getMetaStores();
> }
>
> public TestTablesList(String name, AbstractMetaStoreService metaStore) throws Exception {
> this.metaStore = metaStore;
> this.metaStore.start();
> }
>
> @AfterParam
> public static void stopMetaStores() throws Exception {
> metaStore.stop();
> }
> ```
> Until then we need a way to stop the metastore services at the end of the runs.
> Sidenote: Currently metaStore.stop() does not do anything, since there is no clean way to stop a metastore. So really it is only there to remind us, that we should do something like this.
>
> Do you think we should remove the extra code, and file a jira to do this, when we have either the junit change in a release, or a way to stop a metastore?
> Thanks,
> Peter
>
> Sahil Takiar wrote:
> I think its fine for now, its more of a nit. Regardless of the junit fix, can't all this logic still be pulled into an abstract class?
>
> Isn't stopping a remote metastore consist of just shutting down the process running HMS?
About the abstract class: I try to avoid the abstract classes in tests when possible, because I think the typical usage of the tests is that a developer creates a patch, and a random test starts to break. Then the developer arrives a totally unfamiliar part of the code, and the less abstraction she meets, the more easily can she solve the issue. But To be honest I am not absolutely sure on this decision. I am ready the reopen this discussion after we committed the patches :)
About stopping the metastore: I am not sure that stopping the metastore has any usages in production code. In these tests we start multiple metastore instances, so it would be good if we can stop them after the tests are finished, but the process is still running.
> On Jan. 24, 2018, 5:50 p.m., Sahil Takiar wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
> > Lines 218-221 (patched)
> > <https://reviews.apache.org/r/65264/diff/3/?file=1944443#file1944443line218>
> >
> > It's a bit weird that the embedded metastore and remote metastore throw different exceptions (speaking broadly here since this applies to a lot of the tests).
> >
> > Is there any way to fix that?
>
> Peter Vary wrote:
> Yeah, this "API" does many weird things :)
> Since the consensus on the dev list was, that we do not want to change the API, I do not see how can we solve this issue:
> - Embedded MetaStore should not throw TProtocolException
> - Remote MetaStore API is defined by the hive_metastore.thrift definition which defines this field as "required"
> We can change the IMetaStoreClient implementation, to check this before sending it to the MetaStore server, but that solution is even worse :(
>
> So I would skip this for now. What do you think?
>
> Sahil Takiar wrote:
> Yeah, I was just curious. Its out of scope for this RB. I think making the making the two consistent is justifiable, even if it is backwards incompatible. We can make the change in Hive 3.0.0. Even if it comes after the HMS separation work, would still be good to have a JIRA to track it.
Filed the jira: HIVE-18555
Inconsistent exceptions between different IMetaStoreClient implementations
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/#review196144
-----------------------------------------------------------
On Jan. 25, 2018, 1:01 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65264/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2018, 1:01 p.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18481
> https://issues.apache.org/jira/browse/HIVE-18481
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create IMetaStoreClient tests to cover the table query methods
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetExists.java PRE-CREATION
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesList.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/65264/diff/4/
>
>
> Testing
> -------
>
> Run the created tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
Posted by Sahil Takiar <ta...@gmail.com>.
> On Jan. 24, 2018, 5:50 p.m., Sahil Takiar wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
> > Lines 63 (patched)
> > <https://reviews.apache.org/r/65264/diff/3/?file=1944443#file1944443line63>
> >
> > A lot of the setup and teardown code for all these classes looks pretty similar. Is it worth creating an abstract class (something like `AbstractMetaStoreClientTest`) to encapsulate all the common setup / teardown logic?
> >
> > We don't have to do this for all the JIRAs in HIVE-18371, since most of them are already done. But would be good for future tests.
>
> Peter Vary wrote:
> When we have a junit release with this commit:
> https://github.com/junit-team/junit4/commit/1bf8438b65858565dbb64736bfe13aae9cfc1b5a
>
> Then we can change the code to something like this:
> ```
> @Parameterized.Parameters(name = "{0}")
> public static List<Object[]> getMetaStoreToTest() throws Exception {
> return MetaStoreFactoryForTests.getMetaStores();
> }
>
> public TestTablesList(String name, AbstractMetaStoreService metaStore) throws Exception {
> this.metaStore = metaStore;
> this.metaStore.start();
> }
>
> @AfterParam
> public static void stopMetaStores() throws Exception {
> metaStore.stop();
> }
> ```
> Until then we need a way to stop the metastore services at the end of the runs.
> Sidenote: Currently metaStore.stop() does not do anything, since there is no clean way to stop a metastore. So really it is only there to remind us, that we should do something like this.
>
> Do you think we should remove the extra code, and file a jira to do this, when we have either the junit change in a release, or a way to stop a metastore?
> Thanks,
> Peter
I think its fine for now, its more of a nit. Regardless of the junit fix, can't all this logic still be pulled into an abstract class?
Isn't stopping a remote metastore consist of just shutting down the process running HMS?
> On Jan. 24, 2018, 5:50 p.m., Sahil Takiar wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
> > Lines 218-221 (patched)
> > <https://reviews.apache.org/r/65264/diff/3/?file=1944443#file1944443line218>
> >
> > It's a bit weird that the embedded metastore and remote metastore throw different exceptions (speaking broadly here since this applies to a lot of the tests).
> >
> > Is there any way to fix that?
>
> Peter Vary wrote:
> Yeah, this "API" does many weird things :)
> Since the consensus on the dev list was, that we do not want to change the API, I do not see how can we solve this issue:
> - Embedded MetaStore should not throw TProtocolException
> - Remote MetaStore API is defined by the hive_metastore.thrift definition which defines this field as "required"
> We can change the IMetaStoreClient implementation, to check this before sending it to the MetaStore server, but that solution is even worse :(
>
> So I would skip this for now. What do you think?
Yeah, I was just curious. Its out of scope for this RB. I think making the making the two consistent is justifiable, even if it is backwards incompatible. We can make the change in Hive 3.0.0. Even if it comes after the HMS separation work, would still be good to have a JIRA to track it.
- Sahil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/#review196144
-----------------------------------------------------------
On Jan. 25, 2018, 1:01 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65264/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2018, 1:01 p.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18481
> https://issues.apache.org/jira/browse/HIVE-18481
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create IMetaStoreClient tests to cover the table query methods
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetExists.java PRE-CREATION
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesList.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/65264/diff/4/
>
>
> Testing
> -------
>
> Run the created tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
> On Jan. 24, 2018, 5:50 p.m., Sahil Takiar wrote:
> >
Thanks for the review Sahil!
> On Jan. 24, 2018, 5:50 p.m., Sahil Takiar wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
> > Lines 63 (patched)
> > <https://reviews.apache.org/r/65264/diff/3/?file=1944443#file1944443line63>
> >
> > A lot of the setup and teardown code for all these classes looks pretty similar. Is it worth creating an abstract class (something like `AbstractMetaStoreClientTest`) to encapsulate all the common setup / teardown logic?
> >
> > We don't have to do this for all the JIRAs in HIVE-18371, since most of them are already done. But would be good for future tests.
When we have a junit release with this commit:
https://github.com/junit-team/junit4/commit/1bf8438b65858565dbb64736bfe13aae9cfc1b5a
Then we can change the code to something like this:
```
@Parameterized.Parameters(name = "{0}")
public static List<Object[]> getMetaStoreToTest() throws Exception {
return MetaStoreFactoryForTests.getMetaStores();
}
public TestTablesList(String name, AbstractMetaStoreService metaStore) throws Exception {
this.metaStore = metaStore;
this.metaStore.start();
}
@AfterParam
public static void stopMetaStores() throws Exception {
metaStore.stop();
}
```
Until then we need a way to stop the metastore services at the end of the runs.
Sidenote: Currently metaStore.stop() does not do anything, since there is no clean way to stop a metastore. So really it is only there to remind us, that we should do something like this.
Do you think we should remove the extra code, and file a jira to do this, when we have either the junit change in a release, or a way to stop a metastore?
Thanks,
Peter
> On Jan. 24, 2018, 5:50 p.m., Sahil Takiar wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
> > Lines 218-221 (patched)
> > <https://reviews.apache.org/r/65264/diff/3/?file=1944443#file1944443line218>
> >
> > It's a bit weird that the embedded metastore and remote metastore throw different exceptions (speaking broadly here since this applies to a lot of the tests).
> >
> > Is there any way to fix that?
Yeah, this "API" does many weird things :)
Since the consensus on the dev list was, that we do not want to change the API, I do not see how can we solve this issue:
- Embedded MetaStore should not throw TProtocolException
- Remote MetaStore API is defined by the hive_metastore.thrift definition which defines this field as "required"
We can change the IMetaStoreClient implementation, to check this before sending it to the MetaStore server, but that solution is even worse :(
So I would skip this for now. What do you think?
> On Jan. 24, 2018, 5:50 p.m., Sahil Takiar wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
> > Lines 272 (patched)
> > <https://reviews.apache.org/r/65264/diff/3/?file=1944443#file1944443line272>
> >
> > since only one exception is thrown, can use `expected` annotation.
I have missed this. Thanks!
> On Jan. 24, 2018, 5:50 p.m., Sahil Takiar wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
> > Lines 338 (patched)
> > <https://reviews.apache.org/r/65264/diff/3/?file=1944443#file1944443line338>
> >
> > should we just create a separate class for the list table APIs? they use a different set of tables anyway, and it might be good to split up the different filter tests
Good idea!
Done
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/#review196144
-----------------------------------------------------------
On Jan. 23, 2018, 11:18 a.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65264/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2018, 11:18 a.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18481
> https://issues.apache.org/jira/browse/HIVE-18481
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create IMetaStoreClient tests to cover the table query methods
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/65264/diff/3/
>
>
> Testing
> -------
>
> Run the created tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
Posted by Sahil Takiar <ta...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/#review196144
-----------------------------------------------------------
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
Lines 63 (patched)
<https://reviews.apache.org/r/65264/#comment275648>
A lot of the setup and teardown code for all these classes looks pretty similar. Is it worth creating an abstract class (something like `AbstractMetaStoreClientTest`) to encapsulate all the common setup / teardown logic?
We don't have to do this for all the JIRAs in HIVE-18371, since most of them are already done. But would be good for future tests.
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
Lines 218-221 (patched)
<https://reviews.apache.org/r/65264/#comment275651>
It's a bit weird that the embedded metastore and remote metastore throw different exceptions (speaking broadly here since this applies to a lot of the tests).
Is there any way to fix that?
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
Lines 272 (patched)
<https://reviews.apache.org/r/65264/#comment275652>
since only one exception is thrown, can use `expected` annotation.
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
Lines 338 (patched)
<https://reviews.apache.org/r/65264/#comment275653>
should we just create a separate class for the list table APIs? they use a different set of tables anyway, and it might be good to split up the different filter tests
- Sahil Takiar
On Jan. 23, 2018, 11:18 a.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65264/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2018, 11:18 a.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18481
> https://issues.apache.org/jira/browse/HIVE-18481
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create IMetaStoreClient tests to cover the table query methods
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/65264/diff/3/
>
>
> Testing
> -------
>
> Run the created tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
Posted by Marta Kuczora via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/#review195984
-----------------------------------------------------------
Ship it!
Ship It!
- Marta Kuczora
On Jan. 23, 2018, 11:18 a.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65264/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2018, 11:18 a.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18481
> https://issues.apache.org/jira/browse/HIVE-18481
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create IMetaStoreClient tests to cover the table query methods
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/65264/diff/3/
>
>
> Testing
> -------
>
> Run the created tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
Posted by Adam Szita via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/#review196029
-----------------------------------------------------------
Ship it!
Ship It!
- Adam Szita
On Jan. 23, 2018, 11:18 a.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65264/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2018, 11:18 a.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18481
> https://issues.apache.org/jira/browse/HIVE-18481
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create IMetaStoreClient tests to cover the table query methods
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/65264/diff/3/
>
>
> Testing
> -------
>
> Run the created tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/
-----------------------------------------------------------
(Updated Jan. 23, 2018, 11:18 a.m.)
Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
Changes
-------
Addressed Marta's comment
Bugs: HIVE-18481
https://issues.apache.org/jira/browse/HIVE-18481
Repository: hive-git
Description
-------
Create IMetaStoreClient tests to cover the table query methods
Diffs (updated)
-----
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java PRE-CREATION
Diff: https://reviews.apache.org/r/65264/diff/3/
Changes: https://reviews.apache.org/r/65264/diff/2-3/
Testing
-------
Run the created tests
Thanks,
Peter Vary
Re: Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
> On Jan. 23, 2018, 10:56 a.m., Marta Kuczora wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
> > Lines 285 (patched)
> > <https://reviews.apache.org/r/65264/diff/2/?file=1943734#file1943734line285>
> >
> > Might worth to add a test with empty filter.
Done. Thanks for pointing this out!
- Peter
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/#review195979
-----------------------------------------------------------
On Jan. 23, 2018, 11:18 a.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65264/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2018, 11:18 a.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18481
> https://issues.apache.org/jira/browse/HIVE-18481
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create IMetaStoreClient tests to cover the table query methods
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/65264/diff/3/
>
>
> Testing
> -------
>
> Run the created tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
Posted by Marta Kuczora via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/#review195979
-----------------------------------------------------------
Thanks a lot Peter for the patch.
I looks really good to me, I have only one comment.
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
Lines 285 (patched)
<https://reviews.apache.org/r/65264/#comment275449>
Might worth to add a test with empty filter.
- Marta Kuczora
On Jan. 22, 2018, 3:38 p.m., Peter Vary wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65264/
> -----------------------------------------------------------
>
> (Updated Jan. 22, 2018, 3:38 p.m.)
>
>
> Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
>
>
> Bugs: HIVE-18481
> https://issues.apache.org/jira/browse/HIVE-18481
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Create IMetaStoreClient tests to cover the table query methods
>
>
> Diffs
> -----
>
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/65264/diff/2/
>
>
> Testing
> -------
>
> Run the created tests
>
>
> Thanks,
>
> Peter Vary
>
>
Re: Review Request 65264: HIVE-18481: Create tests for table related
methods (get, list, exists)
Posted by Peter Vary via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65264/
-----------------------------------------------------------
(Updated Jan. 22, 2018, 3:38 p.m.)
Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.
Changes
-------
Addressed checkstyle errors
Bugs: HIVE-18481
https://issues.apache.org/jira/browse/HIVE-18481
Repository: hive-git
Description
-------
Create IMetaStoreClient tests to cover the table query methods
Diffs (updated)
-----
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java PRE-CREATION
Diff: https://reviews.apache.org/r/65264/diff/2/
Changes: https://reviews.apache.org/r/65264/diff/1-2/
Testing
-------
Run the created tests
Thanks,
Peter Vary