-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature][OssFile Connector] Make Oss implement source factory and sink factory #6062
[Feature][OssFile Connector] Make Oss implement source factory and sink factory #6062
Conversation
d014a2f
to
b066ebd
Compare
@@ -29,6 +29,9 @@ | |||
|
|||
<properties> | |||
<hadoop3.version>3.1.4</hadoop3.version> | |||
<hadoop-aliyun.version>3.1.4</hadoop-aliyun.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whether it needs to appear in example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whether it needs to appear in example?
Thanks, I will roll back it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on #5545 , we need to remove the binary and automatically generate it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on #5545 , we need to remove the binary and automatically generate it
I asked other open source projects, but this is not mandatory.
public class LocalFileSinkFactory | ||
implements TableSinkFactory< | ||
SeaTunnelRow, FileSinkState, FileCommitInfo, FileAggregatedCommitInfo> { | ||
public class LocalFileSinkFactory extends BaseMultipleTableFinkSinkFactory { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change related to oss?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change related to oss?
While adding multi-table support for oss, I found that BaseMultipleTableFinkSinkFactory
could be added to make the code cleaner, so I needed to modify the LocalFileSinkFactory
implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change related to oss?
While adding multi-table support for oss, I found that BaseMultipleTableFinkSinkFactory
could be added to make the code cleaner, so I needed to modify the LocalFileSinkFactory
implementation
...nnel-connector-v2-e2e/connector-file-oss-e2e/src/test/resources/excel/fake_to_oss_excel.conf
Outdated
Show resolved
Hide resolved
Co-authored-by: hailin0 <wanghailin@apache.org>
Co-authored-by: hailin0 <wanghailin@apache.org>
…e/src/test/resources/excel/fake_to_oss_excel.conf Co-authored-by: hailin0 <wanghailin@apache.org>
7c5d7de
to
e823425
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Purpose of this pull request
Does this PR introduce any user-facing change?
Yes. For specific information, please refer to the changes in the document.
How was this patch tested?
I tested in my local and all test case is success.
![image](https://private-user-images.githubusercontent.com/32193458/292473479-0558f265-3929-4854-aee6-503146dcf0b2.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjIyODQ1NzcsIm5iZiI6MTcyMjI4NDI3NywicGF0aCI6Ii8zMjE5MzQ1OC8yOTI0NzM0NzktMDU1OGYyNjUtMzkyOS00ODU0LWFlZTYtNTAzMTQ2ZGNmMGIyLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA3MjklMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNzI5VDIwMTc1N1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWI0ZWNlODY5MjZjMDA2MWM0NzVhYzZkYzRmYTBjY2RmMWUwZTUyZDNkN2NjOGQ3ZDY5NWRlYWM1NzA0Y2YxOGEmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.uGhHRC8m90kHOsXJGLTzy_CXNGj7kvPG2IzpZFAkYYg)
Because there is no available OSS environment, I disabled the test cases.
Check list
New License Guide
release-note
.