You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@seatunnel.apache.org by xuankun zheng <zh...@gmail.com> on 2022/03/16 17:02:57 UTC
[DISCUSS]Upgread the version of Kudu and add unit tests.
Hello everyone,
I want to upgrade the version of Kudu from 1.7 to 1.12 and add some unit
tests for Kudu according to the previous discussion on github.
More infomation about the discussion can refer to this PR(
https://github.com/apache/incubator-seatunnel/pull/1459).
And now I want to confirm how about this idea?
Due to Kudu has only supported linux/macOS x86, there may be a little
problem when we run on Windows or Mac M1 aarch64:
- Maven: We need to set a fixed classifier which is different from the
Kudu's offcial example. We will get an error when import on Windows/Mac M1
aarch64 platform due to the ${os.detected.classifier} will specify the
infomation according to our platform, and we can't find the specified
version of jar. All versions of the jar that Kudu has supported can refer
to https://repo1.maven.org/maven2/org/apache/kudu/kudu-binary/1.12.0/.
```xml
<!-- Windows and Mac M1 aarch64 will get error when import -->
<dependency>
<groupId>org.apache.kudu</groupId>
<artifactId>kudu-binary</artifactId>
<version>1.12.0</version>
<classifier>${os.detected.classifier}</classifier>
<scope>test</scope>
</dependency>
<!-- set a fixed classifier which is linux-x86_64 -->
<dependency>
<groupId>org.apache.kudu</groupId>
<artifactId>kudu-binary</artifactId>
<version>1.12.0</version>
<classifier>linux-x86_64</classifier>
<scope>test</scope>
</dependency>
```
- Junit: We may need to upgrade the Junit version from Junit4 to Junit5 in
order to use the new annotation (@EnabledOnOs, which can make the unit test
only run on the specified platform), or we can also use the
assume(condition) in scalatest to achieve this goal.
So how about the idea which are mentioned above?
(PS: i am not so good at English. And this is the first time to use this
form of email, please forgive me if the description is not clear).
Best wishes!
Re: [DISCUSS]Upgread the version of Kudu and add unit tests.
Posted by xuankun zheng <zh...@gmail.com>.
Yeah, add -Dos.detected.classifier=osx-x86_64 is a good idea!
I have made some tests, we need to add -Dos.detected.classifier=osx-x86_64
or -Dos.detected.classifier=linux-x86_64 in some places when people develop
on windows and mac m1 platform using ide.
Take IntelliJ IDEA 2021 as an example:
- For maven import: Build, Execution,Deployment - Build Tools - Maven -
Importing - VM options for importer.
- For maven compile/package with test: Build, Execution,Deployment - Build
Tools - Maven - Runner - VM options for importer).
This should be also updated on the document where relevant to the project's
import and build if we import kudu-binary for test. And I will pay
attention to updating the document.
About JUnit, there may be a little misunderstanding about it. The purpose
of -Dos.detected.classifier=osx-x86_64 /
-Dos.detected.classifier=linux-x86_64 is in order to solve the problem when
import the project. And the purpose of upgrade about JUnit5 is in order to
solve the compatibility about the unit test on windows and mac m1 platform.
Consider this situation, we set -Dos.detected.classifier=osx-x86_64 on mac
m1 while aarch64 architecture hasn't supported by Kudu, and we will import
the kudu-binary-1.12.0-osx-x86_64.jar according to our setting. When we run
the unit test or run maven package with test, we will get an error because
kudu-binary-1.12.0-osx-x86_64.jar cannot run on mac m1.
So I think we need a way to skip the unit test automatically according to
the platform. I have found the Assume in JUnit4 can also achieve this goal,
so we don't have to upgrade to JUnit5. But I think the annotation in JUnit5
is more elegant, and we don't have too many JUnit4 test so far, so I think
this may be a good chance to use JUnit5.
Best wishes !
xuankun zheng
wenjun <we...@apache.org> 於 2022年3月17日 週四 下午8:56寫道：
> +1
>
> Upgrade Kudu from 1.7 to 1.12 looks good to me. As Kirs said, we can
> set the property by -D, so we may don't need to upgrade JUnit now.
>
> Best,
> Wenjun Ruan
>
> On Thu, Mar 17, 2022 at 8:09 PM CalvinKirs <ac...@163.com> wrote:
> >
> > Hi,
> > +1, That makes sense to me. Thanks for your suggestion.
> > but we don't have to explicitly declare the classifier property, when
> the user is in an environment like M1, just pass the property with mvn ....
> -Dos.detected.classifier=osx-x86_64 .
> >
> >
> > Best wishes！
> > Calvin Kirs
> >
> >
> > On 03/17/2022 01:02，xuankun zheng<zh...@gmail.com> wrote：
> > Hello everyone,
> >
> > I want to upgrade the version of Kudu from 1.7 to 1.12 and add some unit
> > tests for Kudu according to the previous discussion on github.
> >
> > More infomation about the discussion can refer to this PR(
> > https://github.com/apache/incubator-seatunnel/pull/1459).
> >
> > And now I want to confirm how about this idea?
> >
> > Due to Kudu has only supported linux/macOS x86, there may be a little
> > problem when we run on Windows or Mac M1 aarch64:
> >
> > - Maven: We need to set a fixed classifier which is different from the
> > Kudu's offcial example. We will get an error when import on Windows/Mac
> M1
> > aarch64 platform due to the ${os.detected.classifier} will specify the
> > infomation according to our platform, and we can't find the specified
> > version of jar. All versions of the jar that Kudu has supported can refer
> > to https://repo1.maven.org/maven2/org/apache/kudu/kudu-binary/1.12.0/.
> >
> > ```xml
> > <!-- Windows and Mac M1 aarch64 will get error when import -->
> > <dependency>
> > <groupId>org.apache.kudu</groupId>
> > <artifactId>kudu-binary</artifactId>
> > <version>1.12.0</version>
> > <classifier>${os.detected.classifier}</classifier>
> > <scope>test</scope>
> > </dependency>
> >
> >
> > <!-- set a fixed classifier which is linux-x86_64 -->
> > <dependency>
> > <groupId>org.apache.kudu</groupId>
> > <artifactId>kudu-binary</artifactId>
> > <version>1.12.0</version>
> > <classifier>linux-x86_64</classifier>
> > <scope>test</scope>
> > </dependency>
> > ```
> >
> > - Junit: We may need to upgrade the Junit version from Junit4 to Junit5
> in
> > order to use the new annotation (@EnabledOnOs, which can make the unit
> test
> > only run on the specified platform), or we can also use the
> > assume(condition) in scalatest to achieve this goal.
> >
> > So how about the idea which are mentioned above?
> > (PS: i am not so good at English. And this is the first time to use this
> > form of email, please forgive me if the description is not clear).
> >
> > Best wishes!
>
Re: [DISCUSS]Upgread the version of Kudu and add unit tests.
Posted by wenjun <we...@apache.org>.
+1
Upgrade Kudu from 1.7 to 1.12 looks good to me. As Kirs said, we can
set the property by -D, so we may don't need to upgrade JUnit now.
Best,
Wenjun Ruan
On Thu, Mar 17, 2022 at 8:09 PM CalvinKirs <ac...@163.com> wrote:
>
> Hi,
> +1, That makes sense to me. Thanks for your suggestion.
> but we don't have to explicitly declare the classifier property, when the user is in an environment like M1, just pass the property with mvn .... -Dos.detected.classifier=osx-x86_64 .
>
>
> Best wishes！
> Calvin Kirs
>
>
> On 03/17/2022 01:02，xuankun zheng<zh...@gmail.com> wrote：
> Hello everyone,
>
> I want to upgrade the version of Kudu from 1.7 to 1.12 and add some unit
> tests for Kudu according to the previous discussion on github.
>
> More infomation about the discussion can refer to this PR(
> https://github.com/apache/incubator-seatunnel/pull/1459).
>
> And now I want to confirm how about this idea?
>
> Due to Kudu has only supported linux/macOS x86, there may be a little
> problem when we run on Windows or Mac M1 aarch64:
>
> - Maven: We need to set a fixed classifier which is different from the
> Kudu's offcial example. We will get an error when import on Windows/Mac M1
> aarch64 platform due to the ${os.detected.classifier} will specify the
> infomation according to our platform, and we can't find the specified
> version of jar. All versions of the jar that Kudu has supported can refer
> to https://repo1.maven.org/maven2/org/apache/kudu/kudu-binary/1.12.0/.
>
> ```xml
> <!-- Windows and Mac M1 aarch64 will get error when import -->
> <dependency>
> <groupId>org.apache.kudu</groupId>
> <artifactId>kudu-binary</artifactId>
> <version>1.12.0</version>
> <classifier>${os.detected.classifier}</classifier>
> <scope>test</scope>
> </dependency>
>
>
> <!-- set a fixed classifier which is linux-x86_64 -->
> <dependency>
> <groupId>org.apache.kudu</groupId>
> <artifactId>kudu-binary</artifactId>
> <version>1.12.0</version>
> <classifier>linux-x86_64</classifier>
> <scope>test</scope>
> </dependency>
> ```
>
> - Junit: We may need to upgrade the Junit version from Junit4 to Junit5 in
> order to use the new annotation (@EnabledOnOs, which can make the unit test
> only run on the specified platform), or we can also use the
> assume(condition) in scalatest to achieve this goal.
>
> So how about the idea which are mentioned above?
> (PS: i am not so good at English. And this is the first time to use this
> form of email, please forgive me if the description is not clear).
>
> Best wishes!
Re:[DISCUSS]Upgread the version of Kudu and add unit tests.
Posted by CalvinKirs <ac...@163.com>.
Hi,
+1, That makes sense to me. Thanks for your suggestion.
but we don't have to explicitly declare the classifier property, when the user is in an environment like M1, just pass the property with mvn .... -Dos.detected.classifier=osx-x86_64 .
Best wishes！
Calvin Kirs
On 03/17/2022 01:02，xuankun zheng<zh...@gmail.com> wrote：
Hello everyone,
I want to upgrade the version of Kudu from 1.7 to 1.12 and add some unit
tests for Kudu according to the previous discussion on github.
More infomation about the discussion can refer to this PR(
https://github.com/apache/incubator-seatunnel/pull/1459).
And now I want to confirm how about this idea?
Due to Kudu has only supported linux/macOS x86, there may be a little
problem when we run on Windows or Mac M1 aarch64:
- Maven: We need to set a fixed classifier which is different from the
Kudu's offcial example. We will get an error when import on Windows/Mac M1
aarch64 platform due to the ${os.detected.classifier} will specify the
infomation according to our platform, and we can't find the specified
version of jar. All versions of the jar that Kudu has supported can refer
to https://repo1.maven.org/maven2/org/apache/kudu/kudu-binary/1.12.0/.
```xml
<!-- Windows and Mac M1 aarch64 will get error when import -->
<dependency>
<groupId>org.apache.kudu</groupId>
<artifactId>kudu-binary</artifactId>
<version>1.12.0</version>
<classifier>${os.detected.classifier}</classifier>
<scope>test</scope>
</dependency>
<!-- set a fixed classifier which is linux-x86_64 -->
<dependency>
<groupId>org.apache.kudu</groupId>
<artifactId>kudu-binary</artifactId>
<version>1.12.0</version>
<classifier>linux-x86_64</classifier>
<scope>test</scope>
</dependency>
```
- Junit: We may need to upgrade the Junit version from Junit4 to Junit5 in
order to use the new annotation (@EnabledOnOs, which can make the unit test
only run on the specified platform), or we can also use the
assume(condition) in scalatest to achieve this goal.
So how about the idea which are mentioned above?
(PS: i am not so good at English. And this is the first time to use this
form of email, please forgive me if the description is not clear).
Best wishes!