Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Feature][Connector-V2] Grok config is supported for file reading #8376

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

[Feature][Connector-V2] Grok config is supported for file reading #8376

wants to merge 6 commits into from

Conversation

corgy-w
Copy link
Contributor

@corgy-w corgy-w commented Dec 24, 2024 •
edited
Loading

Purpose of this pull request

#8103
Currently, only the grok parameter can be configured for a read

Does this PR introduce any user-facing change?

How was this patch tested?

⚠️waiting！！！！！

Check list

@github-actions github-actions bot added dependencies Pull requests that update a dependency file connectors-v2 file labels Dec 24, 2024
@wuchunfu
Copy link
Member

@corgy-w Can you add some commonly used built-in grok expressions?

@corgy-w
Copy link
Contributor Author

corgy-w commented Dec 24, 2024

@corgy-w Can you add some commonly used built-in grok expressions?

Yes, I will complete these together with the tests later. @wuchunfu

@github-actions github-actions bot removed the file label Dec 24, 2024
@github-actions github-actions bot added the file label Dec 24, 2024
@Hisoka-X Hisoka-X linked an issue Dec 25, 2024 that may be closed by this pull request
[Feature][ALL] Global add grok expression support #8103
Open
3 tasks
@Hisoka-X
Copy link
Member

Could you share some details about what it would be like when this feature finished?

@corgy-w
Copy link
Contributor Author

corgy-w commented Dec 25, 2024

Could you share some details about what it would be like when this feature finished?

Wait a moment, I will update doc in next two days

@github-actions github-actions bot removed the dependencies Pull requests that update a dependency file label Dec 25, 2024
hailin0
hailin0 reviewed Dec 25, 2024
View reviewed changes
Copy link
Member

@hailin0 hailin0 left a comment

Choose a reason for hiding this comment

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

Please add e2e

...se/src/test/java/org/apache/seatunnel/connectors/seatunnel/file/reader/ReadStrategyTest.java Outdated
LocalConf localConf = new LocalConf(FS_DEFAULT_NAME_DEFAULT);
xmlReadStrategy.setPluginConfig(pluginConfig);
xmlReadStrategy.init(localConf);
System.out.println("test...................." + xmlFilePath);
Copy link
Member

Choose a reason for hiding this comment

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

Use log.info(...)

Copy link
Contributor Author

@corgy-w corgy-w Dec 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is a test to see ci error , I will revert this commit

...main/java/org/apache/seatunnel/connectors/seatunnel/file/config/BaseSourceConfigOptions.java
@@ -40,6 +41,24 @@ public class BaseSourceConfigOptions {
.noDefaultValue()
.withDescription("The file path of source files");

public static final Option<FilePathRule> FILE_PATH_RULE =
Copy link
Member

Choose a reason for hiding this comment

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

add SinkFactory#optionRule()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry,I should create a darft pr, these issues will be fixed later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hailin0 hailin0 hailin0 left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
connectors-v2 file
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature][ALL] Global add grok expression support
5 participants
@corgy-w @wuchunfu @Hisoka-X @hailin0 @gushang-hub