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.

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][transform] Add a module to set default value for null field #1958

Merged
merged 5 commits into from
Jul 12, 2022

Conversation

Interest1-wyt
Copy link
Contributor

@Interest1-wyt Interest1-wyt commented May 26, 2022

#1963

Purpose of this pull request

Set default value for null field

Check list

@ruanwenjun
Copy link
Member

Please add an issue and describe why you need to add this transform plugin. In fact, I am confusion with the plugin name etl.

@Interest1-wyt Interest1-wyt changed the title [Feature] Add etl for data handler [Feature][transform] Add a module to set default value for null field May 27, 2022
@Interest1-wyt
Copy link
Contributor Author

Interest1-wyt commented May 27, 2022

I wanted to conceive an ETL plugin that contains various functions at first. The null field processing is only a small part of it. After your question, I found that the function of the module I defined was too general, and each plugin of setunnel must have its own Clear functionality, so I changed my code and module name, which now only provides null field handling. Also I have opened an issue for it.

After I commit new code,Dead Link Checker failed,It doesn't seem to be my problem. Can you help me rerun the ci

@Hisoka-X Hisoka-X requested a review from ruanwenjun June 9, 2022 09:33
docs/en/transform/nulltf.md Outdated Show resolved Hide resolved
ruanwenjun
ruanwenjun previously approved these changes Jul 3, 2022
Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

Generally looks good to me, It might be better if you can submit a PR to add e2e case for this plugin.

@ruanwenjun
Copy link
Member

Please fix the scala checkstyle problem.

@Interest1-wyt
Copy link
Contributor Author

OK, I will revise it and submit it later :)

@Interest1-wyt
Copy link
Contributor Author

@Interest1-wyt
Copy link
Contributor Author

Sorry, I accidentally clicked the close button, could you please help to restart the build :)

@Interest1-wyt
Copy link
Contributor Author

Generally looks good to me, It might be better if you can submit a PR to add e2e case for this plugin.
I try to add a test case to the e2e module, but the error "Could not find a valid Docker environment" is displayed when I run it. Running other test methods in e2e also reports this error. Why is this and how can I fix it?
The project runs on the win10 operating system and the docker component is installed.

error

@Interest1-wyt
Copy link
Contributor Author

Generally looks good to me, It might be better if you can submit a PR to add e2e case for this plugin.
I try to add a test case to the e2e module, but the error "Could not find a valid Docker environment" is displayed when I run it. Running other test methods in e2e also reports this error. Why is this and how can I fix it?
The project runs on the win10 operating system and the docker component is installed.

error

Sorry, this is a problem with my local docker installation, which has been solved.

Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

LGTM

@ruanwenjun ruanwenjun requested a review from wuchunfu July 7, 2022 01:47
@CalvinKirs CalvinKirs merged commit cf9707c into apache:dev Jul 12, 2022
@CalvinKirs CalvinKirs added this to the 2.1.3 milestone Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants