GabrielBBaldez commented on PR #11028:
URL: https://github.com/apache/seatunnel/pull/11028#issuecomment-5516270922
@SEZ9 — when you have a moment, would you mind taking another look? Your
`CHANGES_REQUESTED` from 25 Aug is the only thing outstanding now, and it
predates the commits that address it.
Since that review, on head `301cce1ede52`:
- **Pagination** is fixed in code rather than documented around: `pageing`
now fails at startup with `HTTP-03`, covered by a test that fails against the
previous behaviour with `Expected HttpConnectorException to be thrown, but
nothing was thrown`.
- `getSourceClass()` returns `ShopifySource.class`.
- The `${SHOPIFY_ACCESS_TOKEN}` example explains the `-i` substitution it
actually needs, in both locales.
- The options table, the E2E job, and class Javadoc are in from the earlier
round.
- The two base-connector gaps you flagged — no scheme check, and the token
reachable through `HttpParameter`'s `@Data` `toString()` — are filed as #12025
rather than fixed here, since they reach every header-authenticated HTTP
connector.
CI is green on that head, and @DanielLeens re-reviewed it from scratch this
morning.
No rush at all, and thank you again for the original pass — the pagination
point in particular was the one that mattered, and you were right that
documenting it was not enough.
--
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]
