[Feature][Connector-V2] HTTP supports page increase #5477 #5561

xiaofan2022 · 2023-09-26T07:01:56Z

Purpose of this pull request

http supports page increase #5477

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

If any new Jar binary package adding in your PR, please add License Notice according
New License Guide
If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs
If you are contributing the connector code, please check that the following files are updated:
1. Update change log that in connector document. For more details you can refer to connector-v2
2. Update plugin-mapping.properties and add new connector information in it
3. Update the pom file of seatunnel-dist
Update the release-note.

Hisoka-X · 2023-09-26T07:26:31Z

Please add some test case for it

xiaofan2022 · 2023-09-26T07:45:11Z

source {
    Http {
      url = "http://localhost:8080/test/queryData"
      method = "GET"
      format = "json"
      params={
       page: "${page}"
      }
      content_field = "$.data.*"
      pageing={
       total_page_size=20
       page_field=page
       #total_page_field_path="$.totalPage"
       #json_verify_expression="$.hasNext"
       #json_verify_value="false"

      }
      schema = {
        fields {
          name = string
          age = string
        }
      }
    }
}

The configuration has been updated to the http.md document, and the local verification has passed

Hisoka-X · 2023-09-28T02:45:15Z

LICENSE

-seatunnel-transforms-v2/src/main/java/org/apache/seatunnel/transform/sqlengine/zeta/ZetaSQLType.java                                        from https://github.com/JSQLParser/JSqlParser
-seatunnel-transforms-v2/src/main/java/org/apache/seatunnel/transform/sqlengine/zeta/ZetaSQLFilter.java                                      from https://github.com/JSQLParser/JSqlParser
-seatunnel-transforms-v2/src/main/java/org/apache/seatunnel/transform/sqlengine/zeta/ZetaSQLFunction.java                                    from https://github.com/JSQLParser/JSqlParser
+MIT License


Hisoka-X · 2023-09-28T02:46:27Z

docs/en/connector-v2/source/Http.md

+| json_field                     | Config  | No       | -       | This parameter helps you configure the schema,so this parameter must be used with schema.                                                                                                                    |
+| pageing                        | Config  | No       | -       | This parameter is used for paging queries                                                                                                                                                                    |
+| pageing.page_field             | String  | No       | -       | This parameter is used to specify the page field name in the request parameter                                                                                                                               |
+| *pageing.max_page_size*        | Int     | No       | 10000   | Change the parameter to control the maximum number of pages (if using paging please ensure that this value is greater than the target number of pages otherwise it may cause early *data accuracy* problems) |


Suggested change

| *pageing.max_page_size* | Int | No | 10000 | Change the parameter to control the maximum number of pages (if using paging please ensure that this value is greater than the target number of pages otherwise it may cause early *data accuracy* problems) |

| pageing.max_page_size | Int | No | 10000 | Change the parameter to control the maximum number of pages (if using paging please ensure that this value is greater than the target number of pages otherwise it may cause early *data accuracy* problems) |

Please remove * on pageing.max_page_size

Hisoka-X · 2023-09-28T02:52:21Z

docs/en/connector-v2/source/Http.md

+| pageing.json_verify_expression | String  | No       | -       | This parameter is used verify that the condition is met through json path                                                                                                                                    |
+| pageing.json_verify_value      | String  | No       | -       | This parameter must be configured after json_verify_expression is configured (make sure verify value is in this parameter).                                                                                  |


I don't find the code you use these two config, and I don't know what's used for?

These two parameters are primarily used in cases where there is no total page count field in the API response to determine the number of iterations. They allow you to pinpoint a field using a JSON path and then decide whether to continue the task based on whether a specific condition is met (typically, whether the target value is found in json_verify_value).

Hisoka-X · 2023-09-28T02:54:54Z

@xiaofan2022 Also please add e2e test case for this new feature, you can refer https://github.com/apache/seatunnel/tree/dev/seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test.

Hisoka-X · 2023-09-28T02:55:53Z

cc @ruanwenjun @hailin0

Hisoka-X · 2023-10-10T09:35:58Z

LICENSE

@@ -178,15 +178,15 @@
   APPENDIX: How to apply the Apache License to your work.

      To apply the Apache License to your work, attach the following
-      boilerplate notice, with the fields enclosed by brackets "{}"
+      boilerplate notice, with the fields enclosed by brackets "[]"


Please revert this file change.

Hisoka-X · 2023-10-11T09:07:17Z

...ttp-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/config/HttpConfig.java

+            Options.key("json_verify_expression")
+                    .stringType()
+                    .noDefaultValue()
+                    .withDescription("json verify expression ");


The description not clear, I don't know what's mean if I only read json verify expression.

Hisoka-X · 2023-10-11T09:09:10Z

docs/en/connector-v2/source/Http.md

+| json_field                     | Config  | No       | -       | This parameter helps you configure the schema,so this parameter must be used with schema.                                                                                                                    |
+| pageing                        | Config  | No       | -       | This parameter is used for paging queries                                                                                                                                                                    |
+| pageing.page_field             | String  | No       | -       | This parameter is used to specify the page field name in the request parameter                                                                                                                               |
+| *pageing.max_page_size*        | Int     | No       | 10000   | Change the parameter to control the maximum number of pages (if using paging please ensure that this value is greater than the target number of pages otherwise it may cause early *data accuracy* problems) |


Please remove * on pageing.max_page_size

Hisoka-X · 2023-10-11T09:12:13Z

...ttp-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/config/HttpConfig.java

+            Options.key("json_verify_value")
+                    .stringType()
+                    .noDefaultValue()
+                    .withDescription("json verify value ");


json_verify_expression: retrieve and validate target field values using a json_path expression

json_verify_value: verify if the value of the JSON verify expression matches the expected value.

max_page_size: The "pageing.max_page_size" parameter is primarily used to prevent infinite loops when there is no configuration for validation through JSONPath, ensuring that the program can complete its execution even if the termination condition is not triggered.

Hisoka-X · 2023-10-11T09:12:56Z

...ttp-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/config/HttpConfig.java

+            Options.key("page_field")
+                    .stringType()
+                    .defaultValue("page")
+                    .withDescription("page field");


ditto. I think you should copy description from doc to here.

Hisoka-X · 2023-10-11T09:14:23Z

...nnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_page_increase_page_num.conf

+    json_field = {
+                 name = "$.data[*].name"
+                 age = "$.data[*].age"
+       }
+       pageing={
+              total_page_size=2
+              page_field=page
+       }
+       schema = {
+         fields {
+           name = string
+           age = int
+         }
+       }


Please fix format.

Hisoka-X · 2023-10-11T09:14:41Z

...connector-v2-e2e/connector-http-e2e/src/test/resources/http_page_increase_by_expression.conf

+    json_field = {
+                 name = "$.data[*].name"
+                 age = "$.data[*].age"
+       }
+       pageing={
+        json_verify_expression="$.hasNext"
+        json_verify_value="false"
+        page_field=page
+       }
+       schema = {
+         fields {
+           name = string
+           age = int
+         }
+       }


Hisoka-X · 2023-10-11T09:18:15Z

...-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/mockserver-config.json

+            "age": 51
+          }
+        ],
+        "currentPageIndex": 1,


Suggested change

"currentPageIndex": 1,

"currentPageIndex": 2,

Hisoka-X · 2023-10-12T03:01:25Z

After a second consider, I'm a little worry about the scene this PR try to solved. Normally, we can try to use batch size to check we can continue read next page or not.
Eg:

batch size = 100
if read size < 100 : read fininsh.
if read size = 100 : read next page.

Could you support this for default way to check read next page or not? I think this way will more useful and simple.

xiaofan2022 · 2023-10-12T03:58:55Z

After a second consider, I'm a little worry about the scene this PR try to solved. Normally, we can try to use batch size to check we can continue read next page or not. Eg:
batch size = 100
if read size < 100 : read fininsh.
if read size = 100 : read next page.
Could you support this for default way to check read next page or not? I think this way will more useful and simple.

if read size>100? next batch?

Hisoka-X · 2023-10-12T05:43:19Z

if read size>100? next batch?

yes, but if batch size = 100, the read size could not larger than 100 if api not make mistabke.

xiaofan2022 · 2023-10-13T03:12:54Z

size
Does it support user-configurable batch size? If, after processing a batch, it doesn't meet the validation criteria, it proceeds to the next batch in a loop. In this case, I don't see a difference from using 'while (max_limit)'—in fact, batch processing might even result in idle processing.

Hisoka-X · 2023-10-13T03:44:25Z

I don't see a difference from using 'while (max_limit)'—in fact, batch processing might even result in idle processing.

It would not care the page size, imagine that if the total amount of data returned by the API keeps rising, the pagesize size may need to be changed every time a task is run, but by specifying the batchsize, we can always read without changing the job file to read all data.

xiaofan2022 · 2023-10-26T06:44:56Z

I did not change the Engine-Client related code, why ci/cd error

Hisoka-X · 2023-10-26T06:47:06Z

Never mind, this is a bug will be fixed in #5710

Hisoka-X

LGTM, just one comment

Hisoka-X · 2023-10-26T06:50:42Z

...se/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java

+        this.httpParameter
+                .getParams()
+                .put(pageInfo.getPageField(), pageInfo.getPageIndex().toString());
+        //


Suggested change

//

Hisoka-X · 2023-10-26T06:52:24Z

cc @hailin0 @ruanwenjun

EricJoy2048 · 2023-10-26T09:06:17Z

docs/en/connector-v2/source/Http.md

@@ -310,6 +314,35 @@ source {
 - Test data can be found at this link [mockserver-config.json](../../../../seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/mockserver-config.json)
 - See this link for task configuration [http_jsonpath_to_assert.conf](../../../../seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_jsonpath_to_assert.conf).

+  ### pageing


Suggested change

### pageing

### pageing

docs/en/connector-v2/source/Http.md

Hisoka-X

Thanks @xiaofan2022 !

http supports page increase apache#5477

2aed008

Hisoka-X added this to the 2.3.4 milestone Sep 26, 2023

Update LICENSE

93c3cb4

xiaofan2022 and others added 6 commits September 26, 2023 16:13

spotless

49c2f63

Update LICENSE

a4b415c

add apache header

1f2e6db

spotless

6dc524a

spotless

51dfd47

update license header

8fef4b7

Hisoka-X changed the title ~~http supports page increase #5477~~ [Feature][Connector-V2] HTTP supports page increase #5477 Sep 28, 2023

Hisoka-X reviewed Sep 28, 2023

View reviewed changes

Hisoka-X added the feature New feature label Sep 28, 2023

xiaofan2022 and others added 4 commits September 28, 2023 12:02

Update LICENSE

9b491a3

add test

11c7aa5

add test

f5928f7

update

51167f0

Hisoka-X reviewed Oct 11, 2023

View reviewed changes

xiaofan2022 added 3 commits October 11, 2023 21:29

update

c087c71

update

046d62b

update

eb28c3a

Hisoka-X previously approved these changes Oct 26, 2023

View reviewed changes

github-actions bot added approved reviewed labels Oct 26, 2023

delete comment

00dd09d

xiaofan2022 dismissed Hisoka-X’s stale review via 00dd09d October 26, 2023 07:26

github-actions bot removed approved reviewed labels Oct 26, 2023

Hisoka-X previously approved these changes Oct 26, 2023

View reviewed changes

github-actions bot added approved reviewed labels Oct 26, 2023

EricJoy2048 reviewed Oct 26, 2023

View reviewed changes

Update docs/en/connector-v2/source/Http.md

8a246af

EricJoy2048 dismissed Hisoka-X’s stale review via 8a246af October 26, 2023 09:07

github-actions bot removed approved reviewed labels Oct 26, 2023

EricJoy2048 previously approved these changes Oct 26, 2023

View reviewed changes

github-actions bot added approved reviewed labels Oct 26, 2023

Update Http.md

301f99e

EricJoy2048 dismissed their stale review via 301f99e October 26, 2023 09:10

github-actions bot removed approved reviewed labels Oct 26, 2023

EricJoy2048 approved these changes Oct 26, 2023

View reviewed changes

github-actions bot added approved reviewed labels Oct 26, 2023

Hisoka-X approved these changes Oct 26, 2023

View reviewed changes

Hisoka-X merged commit bb180b2 into apache:dev Oct 26, 2023

	\| pageing.max_page_size \| Int \| No \| 10000 \| Change the parameter to control the maximum number of pages (if using paging please ensure that this value is greater than the target number of pages otherwise it may cause early data accuracy problems) \|
	\| pageing.max_page_size \| Int \| No \| 10000 \| Change the parameter to control the maximum number of pages (if using paging please ensure that this value is greater than the target number of pages otherwise it may cause early data accuracy problems) \|

		\| pageing.json_verify_expression \| String \| No \| - \| This parameter is used verify that the condition is met through json path \|
		\| pageing.json_verify_value \| String \| No \| - \| This parameter must be configured after json_verify_expression is configured (make sure verify value is in this parameter). \|

[Feature][Connector-V2] HTTP supports page increase #5477 #5561

[Feature][Connector-V2] HTTP supports page increase #5477 #5561

Conversation

xiaofan2022 commented Sep 26, 2023

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

Hisoka-X commented Sep 26, 2023

xiaofan2022 commented Sep 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hisoka-X commented Sep 28, 2023

Hisoka-X commented Sep 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hisoka-X commented Oct 12, 2023

xiaofan2022 commented Oct 12, 2023

Hisoka-X commented Oct 12, 2023

xiaofan2022 commented Oct 13, 2023

Hisoka-X commented Oct 13, 2023

xiaofan2022 commented Oct 26, 2023

Hisoka-X commented Oct 26, 2023

Hisoka-X left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hisoka-X commented Oct 26, 2023

Choose a reason for hiding this comment

Hisoka-X left a comment

Choose a reason for hiding this comment