SEZ9 commented on PR #11307: URL: https://github.com/apache/seatunnel/pull/11307#issuecomment-5650255252
Thanks @goutamadwant for 0f2c42cbf — the EN/ZH guidance for existing Spark 3.4 deployments is what I was after for PR11307-F8; I'll read through it once the new-head CI finishes. On logging (PR11307-F4): the dependency tree showing only log4j-slf4j-impl 2.17.1 with SLF4J 1.7.36 and no log4j-slf4j2-impl is reassuring. Since the full distribution wasn't rebuilt, could you either confirm that log4j-slf4j2-impl is no longer in the starter logging include list, or share a listing of the logging directory from a build of the Spark 3.5 starter so we can see what actually lands on the classpath? Either one closes this for me. On follow-ups: - Windows launcher (PR11307-F1, PR11307-F5): I'd prefer the fix in this PR. F1 doesn't need a Windows box to confirm — with delayed expansion disabled before the variables are used, the branches compare literal strings and the call target is literal text, so the script can't run a job as written. Reordering so delayed expansion is active where needed, and propagating java's real exit code instead of exiting 0 (F5), is a small change; a full Windows execution test can be a follow-up if you don't have access to one. - Spark 3.5 micro-batch coverage (PR11307-F7): fine as a separate follow-up, provided we open a tracking issue for a dedicated 3.5 streaming test, and either do a one-off manual run of the streaming template on 3.5.8 and note the result here, or adjust the quick-start wording so we aren't pointing 3.5 users at an unexercised path. Still open from the earlier review: the shell launcher executing a command line assembled from job config values (PR11307-F2), the unquoted args and classpath variables in the same script (PR11307-F6), and a note about the runtime-only surfacing of any Catalyst incompatibility from reusing the 3.3 translation layer in the 3.5 starter (PR11307-F3). Could you say whether you plan to address those here or how you'd like to handle them? I'll do a final pass once CI reports on the new head. <!-- streview-comment:1004 --> -- 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]
