You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@seatunnel.apache.org by GitBox <gi...@apache.org> on 2022/10/05 20:01:44 UTC
[GitHub] [incubator-seatunnel] TyrantLucifer opened a new pull request, #2980: [Bug][Connector-V2] Fix the bug of incorrect path in windows environment
TyrantLucifer opened a new pull request, #2980:
URL: https://github.com/apache/incubator-seatunnel/pull/2980
<!--
Thank you for contributing to SeaTunnel! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
## Contribution Checklist
- Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/incubator-seatunnel/issues).
- Name the pull request in the form "[Feature] [component] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
- Minor fixes should be named following this pattern: `[hotfix] [docs] Fix typo in README.md doc`.
-->
## Purpose of this pull request
fix #2979
<!-- Describe the purpose of this pull request. For example: This pull request adds checkstyle plugin.-->
## Check list
* [x] Code changed are covered with tests, or it does not need tests for reason:
* [ ] If any new Jar binary package adding in your PR, please add License Notice according
[New License Guide](https://github.com/apache/incubator-seatunnel/blob/dev/docs/en/contribution/new-license.md)
* [ ] If necessary, please update the documentation to describe the new feature. https://github.com/apache/incubator-seatunnel/tree/dev/docs
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] Hisoka-X merged pull request #2980: [Bug][Connector-V2][File] Fix the bug of incorrect path in windows environment
Posted by GitBox <gi...@apache.org>.
Hisoka-X merged PR #2980:
URL: https://github.com/apache/incubator-seatunnel/pull/2980
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ashulin commented on a diff in pull request #2980: [Bug][Connector-V2] Fix the bug of incorrect path in windows environment
Posted by GitBox <gi...@apache.org>.
ashulin commented on code in PR #2980:
URL: https://github.com/apache/incubator-seatunnel/pull/2980#discussion_r985720335
##########
seatunnel-connectors-v2/connector-file/connector-file-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/sink/writer/AbstractWriteStrategy.java:
##########
@@ -237,7 +236,7 @@ public void beginTransaction(Long checkpointId) {
*/
public List<String> getTransactionIdFromStates(List<FileSinkState> fileStates) {
String[] pathSegments = new String[]{textFileSinkConfig.getPath(), Constant.SEATUNNEL, jobId};
- String jobDir = String.join(File.separator, pathSegments) + "/";
+ String jobDir = String.join("/", pathSegments) + "/";
Review Comment:
Please change FileSystemUtils#getFileSystem
> FileSystem fileSystem = FileSystem.get(new File(path).toPath().toUri(), CONF);
you can see https://stackoverflow.com/questions/51352404/how-can-i-get-a-proper-uri-in-java-windows
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on a diff in pull request #2980: [Bug][Connector-V2] Fix the bug of incorrect path in windows environment
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on code in PR #2980:
URL: https://github.com/apache/incubator-seatunnel/pull/2980#discussion_r985659398
##########
seatunnel-connectors-v2/connector-file/connector-file-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/sink/writer/AbstractWriteStrategy.java:
##########
@@ -237,7 +236,7 @@ public void beginTransaction(Long checkpointId) {
*/
public List<String> getTransactionIdFromStates(List<FileSinkState> fileStates) {
String[] pathSegments = new String[]{textFileSinkConfig.getPath(), Constant.SEATUNNEL, jobId};
- String jobDir = String.join(File.separator, pathSegments) + "/";
+ String jobDir = String.join("/", pathSegments) + "/";
Review Comment:
Yup, you are right. Good idea，yyds 👍
##########
seatunnel-connectors-v2/connector-file/connector-file-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/sink/writer/AbstractWriteStrategy.java:
##########
@@ -237,7 +236,7 @@ public void beginTransaction(Long checkpointId) {
*/
public List<String> getTransactionIdFromStates(List<FileSinkState> fileStates) {
String[] pathSegments = new String[]{textFileSinkConfig.getPath(), Constant.SEATUNNEL, jobId};
- String jobDir = String.join(File.separator, pathSegments) + "/";
+ String jobDir = String.join("/", pathSegments) + "/";
Review Comment:
> `File.separator` is supported for window, whether it is the problem of the following "/"?
I will do a check. Thanks for your good advice.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ashulin commented on a diff in pull request #2980: [Bug][Connector-V2] Fix the bug of incorrect path in windows environment
Posted by GitBox <gi...@apache.org>.
ashulin commented on code in PR #2980:
URL: https://github.com/apache/incubator-seatunnel/pull/2980#discussion_r985720335
##########
seatunnel-connectors-v2/connector-file/connector-file-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/sink/writer/AbstractWriteStrategy.java:
##########
@@ -237,7 +236,7 @@ public void beginTransaction(Long checkpointId) {
*/
public List<String> getTransactionIdFromStates(List<FileSinkState> fileStates) {
String[] pathSegments = new String[]{textFileSinkConfig.getPath(), Constant.SEATUNNEL, jobId};
- String jobDir = String.join(File.separator, pathSegments) + "/";
+ String jobDir = String.join("/", pathSegments) + "/";
Review Comment:
Please change FileSystemUtils#getFileSystem
> FileSystem fileSystem = FileSystem.get(new File(path).toPath().toUri(), CONF);
you can see https://stackoverflow.com/questions/51352404/how-can-i-get-a-proper-uri-in-java-windows
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on a diff in pull request #2980: [Bug][Connector-V2] Fix the bug of incorrect path in windows environment
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on code in PR #2980:
URL: https://github.com/apache/incubator-seatunnel/pull/2980#discussion_r985634997
##########
seatunnel-connectors-v2/connector-file/connector-file-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/sink/writer/AbstractWriteStrategy.java:
##########
@@ -237,7 +236,7 @@ public void beginTransaction(Long checkpointId) {
*/
public List<String> getTransactionIdFromStates(List<FileSinkState> fileStates) {
String[] pathSegments = new String[]{textFileSinkConfig.getPath(), Constant.SEATUNNEL, jobId};
- String jobDir = String.join(File.separator, pathSegments) + "/";
+ String jobDir = String.join("/", pathSegments) + "/";
Review Comment:
Because the `File.separator` that in Windows is `\`, so the path be joined it will be like this:
`/tmp/seatunnel/dist\xxxx.txt`
this path hdfs protocol is not recognized, I have tried to change the path to the windows format like `\tmp\seatunnel\dist\xxx.txt`, but the hdfs protocol still can not recognize. So in order to run the test cases related to the file connector in the windows environment I decided to change it.
The purpose of this pr is to be able to debug better in the windows environment, there is no impact on the normal function.
If you think it is not necessary, we can close it.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on pull request #2980: [Bug][Connector-V2] Fix the bug of incorrect path in windows environment
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on PR #2980:
URL: https://github.com/apache/incubator-seatunnel/pull/2980#issuecomment-1265256328
> Can UT be added?
config file:
```hocon
#
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
env {
# You can set spark configuration here
spark.app.name = "SeaTunnel"
spark.executor.instances = 1
spark.executor.cores = 1
spark.executor.memory = "1g"
spark.master = local
job.mode = "BATCH"
}
source {
FakeSource {
schema = {
fields {
c_map = "map<string, string>"
c_array = "array<int>"
c_string = string
c_boolean = boolean
c_tinyint = tinyint
c_smallint = smallint
c_int = int
c_bigint = bigint
c_float = float
c_double = double
c_bytes = bytes
c_date = date
c_decimal = "decimal(30, 8)"
c_timestamp = timestamp
c_row = {
c_map = "map<string, string>"
c_array = "array<int>"
c_string = string
c_boolean = boolean
c_tinyint = tinyint
c_smallint = smallint
c_int = int
c_bigint = bigint
c_float = float
c_double = double
c_bytes = bytes
c_date = date
c_decimal = "decimal(30, 8)"
c_timestamp = timestamp
}
}
}
}
# If you would like to get more information about how to configure seatunnel and see full list of source plugins,
# please go to https://seatunnel.apache.org/docs/flink/configuration/source-plugins/Fake
}
transform {
# If you would like to get more information about how to configure seatunnel and see full list of transform plugins,
# please go to https://seatunnel.apache.org/docs/flink/configuration/transform-plugins/Sql
}
sink {
LocalFile {
path="/tmp/seatunnel/dist/text/"
row_delimiter="\n"
partition_dir_expression="${k0}=${v0}"
is_partition_field_write_in_file=true
file_name_expression="${transactionId}_${now}"
file_format="text"
filename_time_format="yyyy.MM.dd"
is_enable_transaction=true
}
# If you would like to get more information about how to configure seatunnel and see full list of sink plugins,
# please go to https://seatunnel.apache.org/docs/connector-v2/sink/File
}
```
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ashulin commented on a diff in pull request #2980: [Bug][Connector-V2] Fix the bug of incorrect path in windows environment
Posted by GitBox <gi...@apache.org>.
ashulin commented on code in PR #2980:
URL: https://github.com/apache/incubator-seatunnel/pull/2980#discussion_r985649601
##########
seatunnel-connectors-v2/connector-file/connector-file-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/sink/writer/AbstractWriteStrategy.java:
##########
@@ -237,7 +236,7 @@ public void beginTransaction(Long checkpointId) {
*/
public List<String> getTransactionIdFromStates(List<FileSinkState> fileStates) {
String[] pathSegments = new String[]{textFileSinkConfig.getPath(), Constant.SEATUNNEL, jobId};
- String jobDir = String.join(File.separator, pathSegments) + "/";
+ String jobDir = String.join("/", pathSegments) + "/";
Review Comment:
You can replace the `/` of the `path` sink option with `File.separator`
##########
seatunnel-connectors-v2/connector-file/connector-file-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/sink/writer/AbstractWriteStrategy.java:
##########
@@ -237,7 +236,7 @@ public void beginTransaction(Long checkpointId) {
*/
public List<String> getTransactionIdFromStates(List<FileSinkState> fileStates) {
String[] pathSegments = new String[]{textFileSinkConfig.getPath(), Constant.SEATUNNEL, jobId};
- String jobDir = String.join(File.separator, pathSegments) + "/";
+ String jobDir = String.join("/", pathSegments) + "/";
Review Comment:
You can replace the `/` of the `path` sink option with `File.separator`.
Perhaps this is a more general solution
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] ashulin commented on a diff in pull request #2980: [Bug][Connector-V2] Fix the bug of incorrect path in windows environment
Posted by GitBox <gi...@apache.org>.
ashulin commented on code in PR #2980:
URL: https://github.com/apache/incubator-seatunnel/pull/2980#discussion_r985625225
##########
seatunnel-connectors-v2/connector-file/connector-file-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/sink/writer/AbstractWriteStrategy.java:
##########
@@ -237,7 +236,7 @@ public void beginTransaction(Long checkpointId) {
*/
public List<String> getTransactionIdFromStates(List<FileSinkState> fileStates) {
String[] pathSegments = new String[]{textFileSinkConfig.getPath(), Constant.SEATUNNEL, jobId};
- String jobDir = String.join(File.separator, pathSegments) + "/";
+ String jobDir = String.join("/", pathSegments) + "/";
Review Comment:
`File.separator` is supported for window, whether it is the problem of the following "/";
##########
seatunnel-connectors-v2/connector-file/connector-file-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/file/sink/writer/AbstractWriteStrategy.java:
##########
@@ -237,7 +236,7 @@ public void beginTransaction(Long checkpointId) {
*/
public List<String> getTransactionIdFromStates(List<FileSinkState> fileStates) {
String[] pathSegments = new String[]{textFileSinkConfig.getPath(), Constant.SEATUNNEL, jobId};
- String jobDir = String.join(File.separator, pathSegments) + "/";
+ String jobDir = String.join("/", pathSegments) + "/";
Review Comment:
`File.separator` is supported for window, whether it is the problem of the following "/"?
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org