You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by "mindlesscloud (via GitHub)" <gi...@apache.org> on 2023/02/02 15:19:33 UTC

[GitHub] [incubator-devlake] mindlesscloud opened a new pull request, #4301: feat: enhance the plugin `customize`

mindlesscloud opened a new pull request, #4301:
URL: https://github.com/apache/incubator-devlake/pull/4301

   ### Summary
   Enhance the plugin `customize` so that it can support the creation of multiple types of fields.
   For now, 5 types were supported:
   - varchar(255)
   - text
   - bigint
   - float
   - timestamp
   Please refer to the swagger docs for details
   
   ### Does this close any open issues?
   relate to #3840 
   
   ### Screenshots
   ![image](https://user-images.githubusercontent.com/8455907/216365028-b3360062-d647-4874-b59b-45918540ad3d.png)
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] warren830 merged pull request #4301: feat: enhance the plugin `customize`

Posted by "warren830 (via GitHub)" <gi...@apache.org>.
warren830 merged PR #4301:
URL: https://github.com/apache/incubator-devlake/pull/4301


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #4301: feat: enhance the plugin `customize`

Posted by "warren830 (via GitHub)" <gi...@apache.org>.
warren830 commented on code in PR #4301:
URL: https://github.com/apache/incubator-devlake/pull/4301#discussion_r1116525433


##########
backend/helpers/e2ehelper/data_flow_tester.go:
##########
@@ -136,7 +136,7 @@ func (t *DataFlowTester) ImportCsvIntoRawTable(csvRelPath string, rawTableName s
 
 // ImportCsvIntoTabler imports records from specified csv file into target tabler, note that existing data would be deleted first.
 func (t *DataFlowTester) ImportCsvIntoTabler(csvRelPath string, dst schema.Tabler) {
-	csvIter := pluginhelper.NewCsvFileIterator(csvRelPath)
+	csvIter, _ := pluginhelper.NewCsvFileIterator(csvRelPath)

Review Comment:
   Same as above



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] mindlesscloud commented on a diff in pull request #4301: feat: enhance the plugin `customize`

Posted by "mindlesscloud (via GitHub)" <gi...@apache.org>.
mindlesscloud commented on code in PR #4301:
URL: https://github.com/apache/incubator-devlake/pull/4301#discussion_r1116655674


##########
backend/helpers/pluginhelper/csv_file_iterator.go:
##########
@@ -67,21 +74,30 @@ func (ci *CsvFileIterator) Close() {
 
 // HasNext returns a boolean to indicate whether there was any row to be `Fetch`
 func (ci *CsvFileIterator) HasNext() bool {
+	hasNext, err := ci.HasNextWithError()
+	if err != nil {
+		panic(err)

Review Comment:
   No, if you wish the error to be returned instead of panic, please use the new function `HasNextWithError`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] mindlesscloud commented on a diff in pull request #4301: feat: enhance the plugin `customize`

Posted by "mindlesscloud (via GitHub)" <gi...@apache.org>.
mindlesscloud commented on code in PR #4301:
URL: https://github.com/apache/incubator-devlake/pull/4301#discussion_r1116644751


##########
backend/helpers/e2ehelper/data_flow_tester.go:
##########
@@ -119,7 +119,7 @@ func NewDataFlowTester(t *testing.T, pluginName string, pluginMeta plugin.Plugin
 
 // ImportCsvIntoRawTable imports records from specified csv file into target raw table, note that existing data would be deleted first.
 func (t *DataFlowTester) ImportCsvIntoRawTable(csvRelPath string, rawTableName string) {
-	csvIter := pluginhelper.NewCsvFileIterator(csvRelPath)
+	csvIter, _ := pluginhelper.NewCsvFileIterator(csvRelPath)

Review Comment:
   As a testing helper function, it could throw panic if something went wrong. However, as a function called by the plugin, it would be better to return an error and let the caller decides how to handle it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #4301: feat: enhance the plugin `customize`

Posted by "warren830 (via GitHub)" <gi...@apache.org>.
warren830 commented on code in PR #4301:
URL: https://github.com/apache/incubator-devlake/pull/4301#discussion_r1116527330


##########
backend/helpers/pluginhelper/csv_file_test.go:
##########
@@ -32,7 +32,7 @@ func TestExampleCsvFile(t *testing.T) {
 	writer.Write([]string{"123", "foobar", `{"url": "https://example.com"}`, "2022-05-05 09:56:43.438000000"})
 	writer.Close()
 
-	iter := NewCsvFileIterator(filename)
+	iter, _ := NewCsvFileIterator(filename)

Review Comment:
   Same problem



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #4301: feat: enhance the plugin `customize`

Posted by "warren830 (via GitHub)" <gi...@apache.org>.
warren830 commented on code in PR #4301:
URL: https://github.com/apache/incubator-devlake/pull/4301#discussion_r1116527197


##########
backend/helpers/pluginhelper/csv_file_iterator.go:
##########
@@ -67,21 +74,30 @@ func (ci *CsvFileIterator) Close() {
 
 // HasNext returns a boolean to indicate whether there was any row to be `Fetch`
 func (ci *CsvFileIterator) HasNext() bool {
+	hasNext, err := ci.HasNextWithError()
+	if err != nil {
+		panic(err)

Review Comment:
   Do we also need to change this to return err?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #4301: feat: enhance the plugin `customize`

Posted by "warren830 (via GitHub)" <gi...@apache.org>.
warren830 commented on code in PR #4301:
URL: https://github.com/apache/incubator-devlake/pull/4301#discussion_r1116525326


##########
backend/helpers/e2ehelper/data_flow_tester.go:
##########
@@ -119,7 +119,7 @@ func NewDataFlowTester(t *testing.T, pluginName string, pluginMeta plugin.Plugin
 
 // ImportCsvIntoRawTable imports records from specified csv file into target raw table, note that existing data would be deleted first.
 func (t *DataFlowTester) ImportCsvIntoRawTable(csvRelPath string, rawTableName string) {
-	csvIter := pluginhelper.NewCsvFileIterator(csvRelPath)
+	csvIter, _ := pluginhelper.NewCsvFileIterator(csvRelPath)

Review Comment:
   Do we need to care about this err?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org