MonkeyCanCode commented on PR #545: URL: https://github.com/apache/polaris/pull/545#issuecomment-2587719741
> @MonkeyCanCode from a first glance, I note that the original ask in #21 was to move the tests to pytests, but your PR has actually 3 (unrelated?) changes: > > 1. Upgrade to python 3.13 > 2. Fix running tests locally > 3. Move to pytests > > Would it be possible to split this PR in 3? In particular I'm worried about the complete redesign of the dockerfile. > > Also, the removal of `regtests/run_spark_sql.sh` seems problematic to me: it's used to run Spark locally for adhoc testing against a local running Polaris server: > > https://github.com/apache/polaris/blob/9377aa731a64d6262dc7cc177d8a457d14c46f0a/regtests/README.md?plain=1#L105-L111 Thanks for the quick check. Yes, I can split them into 3 diff PRs later this week. So the run_spark_sql.sh won't be needed with this PR as it got refactor into running the same thing in pytest. Also, I removed the part for downloading spark distribution and installed it via pip poetry instead. But yeah, let me do them in diff PR. Also, one more thing is python 3.8 is EOL since 2024-10-07. Should we bump the minimal support version to 3.9 instead (EOL at 2025-10)? -- 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]
