GabrielBBaldez commented on PR #11028:
URL: https://github.com/apache/seatunnel/pull/11028#issuecomment-5413836142
Two of these were mine and are fixed in `664872f4`.
**E2E.** Added `shopify_json_to_assert.conf` and its mockserver route, wired
into `HttpIT` beside the other connectors. The mock matches only when
`X-Shopify-Access-Token` is present, so the job also covers the one behaviour
this connector adds over a plain HTTP source — if `buildWithConfig` stops
injecting the header the request no longer matches and the test fails. It uses
`content_field = "$.products.*"`, mirroring the documented configuration, since
the Admin REST API answers with `{"products": [...]}` rather than a bare array.
**Options table.** `headers` and `json_filed_missed_return_null` were
missing from both `en` and `zh` while the option rule accepts them. Added, with
a `headers` section noting that `access_token` already populates the token
header.
On the rest, what I found checking the tree rather than just this diff:
**Pagination is real and worse than one dropped argument.**
`HttpSource.createReader` passes eight arguments; the five-argument constructor
fills the remainder with `null, false, 0L`, so `binaryMode` and
`binaryChunkSize` go with `pageInfo`.
But passing them through would not give working pagination here.
`HttpSourceReader` resolves the next cursor with
`JsonPath.read(pageCursorResponseField)` against the response **body**, and the
Admin REST API returns its cursor in the `Link` **header**. Shopify also
retired page-number paging for REST, so the non-cursor mode would re-request
page one indefinitely rather than fail. Enabling the inherited pagination would
replace a visible limitation with a silent wrong answer.
So I documented the limitation instead of half-fixing it — `## Pagination`
in both locales now states that `pageing` is accepted and ignored, and why.
Three ways forward, and the choice is yours rather than mine:
1. Teach `connector-http-base` to read a `Link`-header cursor, then wire it
here. Benefits every connector against a header-cursor API; a separate PR.
2. Keep the documented limitation for now and drop `pageing` from this
connector's option rule, so a user setting it gets a validation error instead
of silence.
3. Shopify-specific pagination inside this module, diverging from the
siblings.
**The remaining points reproduce across the connector family, so I left them
alone rather than fixing Shopify into an exception:** `getSourceClass()` is
absent from the Klaviyo, Jira and Notion factories too; the `@Data` `toString`
exposure is on `HttpParameter` in `connector-http-base`, not on
`ShopifySourceParameter`, and reaches every connector that sets an auth header;
no HTTP connector validates the URL scheme; and `connector-http-base` has no
`429`/`Retry-After` handling at all. Happy to take any of them, here or
separately — say which.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
