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

[Improve][Connector] Change path_list option setting using concating string with comma , to string list for FtpConnector #424

Open
1 of 2 tasks
Jake-00 opened this issue Feb 24, 2023 · 7 comments

Comments

@Jake-00
Copy link
Contributor

Jake-00 commented Feb 24, 2023

Description

For FtpConnector, currently we can specify directories to read by setting configuration json file using option path_list, but it can't specify several files' path to read.

Suggetion:

  1. currently path_list specify a string contains path list, it is better to change to specify a string list like below
  2. update documentation of FtpCoinnector
"reader": {
      "path_list": ["/data/json/upload1/test1.json", "/data/json/upload2/"]
}

BitSail Component or Code Module

BitSail Connector

Are you willing to submit PR?

  • Yes, I am willing to submit a PR!

Code of Conduct

@Jake-00
Copy link
Contributor Author

Jake-00 commented Feb 25, 2023

This issue maybe good for beginner.

@hk-lrzy
Copy link
Collaborator

hk-lrzy commented Feb 28, 2023

@Jake-00 Thanks for your issue, you suggestion is we just not support directories right, we can use file path directly?

@Jake-00
Copy link
Contributor Author

Jake-00 commented Feb 28, 2023

@hk-lrzy To read directories already support, but to read multiple specified files do not support yet.

@hk-lrzy
Copy link
Collaborator

hk-lrzy commented Mar 2, 2023

@hk-lrzy To read directories already support, but to read multiple specified files do not support yet.

I see, can you use same parameter to support? like we check the path is file or directory, if directory we support to list it and if it is file we support read it directly.

I don't want to add parameter because user need to know use which one, it's not necessary for user.

@Jake-00
Copy link
Contributor Author

Jake-00 commented Mar 2, 2023

Sounds more reasonable, shall I change the issue description?

@hk-lrzy
Copy link
Collaborator

hk-lrzy commented Mar 3, 2023

Sounds more reasonable, shall I change the issue description?

Sure~

@Jake-00
Copy link
Contributor Author

Jake-00 commented Mar 19, 2023

When diving deeper in FtpConnector, it already supports to read specified files and directories. Issue description is wrong and I change the description of this issue. Could we add tag good-first-issue?

@Jake-00 Jake-00 changed the title [Improve][Connector] Support to read specified files rather than just directories for FtpConnector [Improve][Connector] Change path_list option setting using concating string with comma , to string list for FtpConnector Mar 19, 2023
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

No branches or pull requests

2 participants