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

[ISSUE #4635] Implement the function of file source connector #4640

Conversation

HarshSawarkar
Copy link
Contributor

Fixes #4635

Motivation

File source connector class methods were not implemented.

Modifications

Successfully implemented start(), stop(), poll() methods of file source connector class.

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

@HarshSawarkar HarshSawarkar deleted the 4635-Implement-the-function-of-File-Source-Connector branch December 13, 2023 11:04
@HarshSawarkar HarshSawarkar restored the 4635-Implement-the-function-of-File-Source-Connector branch December 13, 2023 11:06
@HarshSawarkar HarshSawarkar reopened this Dec 13, 2023
@harshithasudhakar harshithasudhakar changed the title 4635 implement the function of file source connector [ISSUE #4635] Implement the function of file source connector Dec 13, 2023
Copy link
Member

@harshithasudhakar harshithasudhakar left a comment

Choose a reason for hiding this comment

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

LGTM

@harshithasudhakar
Copy link
Member

@HarshSawarkar Please fix the checkstyle errors.

@VishalMCF
Copy link
Contributor

@HarshSawarkar I have a general question to increase my understanding of the project. I want to know why there was a need to change the EtcdMetaService and how it is related to the FileSourceConnector.

@HarshSawarkar
Copy link
Contributor Author

@HarshSawarkar I have a general question to increase my understanding of the project. I want to know why there was a need to change the EtcdMetaService and how it is related to the FileSourceConnector.

There is no relation between these two classes.EtcdMetaService class was appended by mistake.

…entmesh/meta/etcd/service/EtcdMetaService.java
@VishalMCF
Copy link
Contributor

VishalMCF commented Dec 13, 2023

@HarshSawarkar cool bro. :) 😎 But why that file is being shown as deleted now? It should not be present entirely in your commit.

@HarshSawarkar
Copy link
Contributor Author

@HarshSawarkar cool bro. :) 😎 But why that file is being shown as deleted now? It should not be present entirely in your commit.

I don't know I tried to roll back changes but the file remain intacted so I deleted it.

@harshithasudhakar
Copy link
Member

harshithasudhakar commented Dec 13, 2023

You can try re-uploading the file from the latest master branch to your branch

@HarshSawarkar cool bro. :) 😎 But why that file is being shown as deleted now? It should not be present entirely in your commit.

I don't know I tried to roll back changes but the file remain intacted so I deleted it.

Copy link
Member

@Pil0tXia Pil0tXia left a comment

Choose a reason for hiding this comment

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

Please mark file source connector as completed at eventmesh-connectors/README.md and eventmesh-connectors/README_CN.md.

@HarshSawarkar
Copy link
Contributor Author

Please mark file source connector as completed at eventmesh-connectors/README.md and eventmesh-connectors/README_CN.md.

It's already marked as completed in 'eventmesh-connectors/README.md'.Here's the screenshot for reference
image

return null;
List<ConnectRecord> connectRecords = new ArrayList<>();
try {
BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8));
Copy link
Member

Choose a reason for hiding this comment

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

Can BufferedReader object be set as a global variable? Additionally, in your current code, BufferedReader has a memory leak problem.

@@ -45,7 +63,8 @@ public Class<? extends Config> configClass() {
public void init(Config config) throws Exception {
// init config for hdfs source connector
this.sourceConfig = (FileSourceConfig) config;

this.filePath = buildFilePath();
this.fileName = buildFileName();
Copy link
Member

Choose a reason for hiding this comment

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

Could the file name and file path be directly configured in the configuration file? Additionally, could unnecessary configurations in the configuration file be removed?

@pandaapo
Copy link
Member

@HarshSawarkar cool bro. :) 😎 But why that file is being shown as deleted now? It should not be present entirely in your commit.

I don't know I tried to roll back changes but the file remain intacted so I deleted it.

@HarshSawarkar @VishalMCF
Because this branch was created based on the branch of #4636, there were modifications and deletion of EtcdMetaService that should not have occurred. If this PR is merged, EtcdMetaService in the source code will be deleted. This is not right.

You should create a development branch based on the latest master branch that is consistent with the source code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Implement the function of File Source Connector
5 participants