Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
dongjoon-hyun commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2045379643 Thank you so much for making this PR and letting us conclude this topic, @pan3793 ! -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
pan3793 closed pull request #45852: Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait URL: https://github.com/apache/spark/pull/45852 -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
pan3793 commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2044429278 Thanks @dongjoon-hyun and all participate in this discussion, I think we already reached a consensus to promote `–wait` to all service for consistency, created SPARK-47778 and close this PR then. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
dongjoon-hyun commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2043258249 Hi, @pan3793 and all. Shall we conclude this thread? Given the discussion, I believe we can close this PR and move forward to generalize `--wait` for all services (if someone wants to implement that for consistency.) -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
HyukjinKwon commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2041222711 I haven't looked through the code very closely but at high level seems fine to have that from a cursory look. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
pan3793 commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2041131686 @HyukjinKwon so you are good to promote the `--wait` to all services, right? -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
pan3793 commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2041131626 @HyukjinKwon so you are good to promote the `--wait` to all services, right? -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
HyukjinKwon commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2038584100 The examples you mentioned are mostly for development purpose. The examples I provided are more for user-facing. I think this `--wait` is the similar case. I think it's fine to add an easier option for those options users are expected to use often. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
pan3793 commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2037913897 @grundprinzip I appreciate your great contributions to Spark Connect and improving development efficiency, I just started exploring this new feature. I strongly agree that we should pay attention to improving development efficiency. But let's evaluate it relatively carefully for user-facing changes. Spark is a great product that has been brilliant for over a decade, we should respect the existing services, tools, and user habits when making user-facing changes, and I believe a consistent user experience is an important element for such a great product. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
pan3793 commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2037707135 > Mind pointing out the examples explicitly? @HyukjinKwon To clarify, the assumption is limited to dev/test purposes, not to end users. Examples: - SPARK_PREPEND_CLASSES - SPARK_GENERATE_GOLDEN_FILES - SKIP_MIMA - SKIP_RDOC - SKIP_PYTHONDOC - SKIP_SQLDOC @gengliangwang I think it's also good if the community reaches a consensus to promote the `--wait` to all services, like SHS, STS. Another simple way is mentioning `SPARK_NO_DAEMONIZE` in `sbin/` scripts' help message. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
HyukjinKwon commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2035811556 > There are several server components in Spark, like Thrift Server, History Server, Master, Worker, only adding --wait on the Connect Server makes things inconsistent, while env vars approach is adopted widely in Spark for different development/test purposes. Mind pointing out the examples explicitly? -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
gengliangwang commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2035714026 @pan3793 @dongjoon-hyun @grundprinzip How about creating an umbrella jira to support option `--wait` in the start scripts under `sbin/` dir? 1. There are just a few of them. 2. Having such a command line option would be helpful. Many users just run `./sbin/sript -h` to see the available options and they may never be aware of the env variables. 3. These new tasks would be great for new Spark developers. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
pan3793 commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2035283307 > "the other servers don't have this option" @grundprinzip That's the point, it violates the consistency. The option is exposed to end users, from the user's point of view, if `./sbin/start-connect-server.sh --wait` works, why `./sbin/start-thriftserver.sh --wait` does not work? What if I want to add a `--debug` option to `./sbin/start-thriftserver.sh --debug`, things go to mess without restrictions. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
grundprinzip commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2035258484 I absolutely appreciate the feedback. I spend most of my time developing Spark Connect and I can only attest that this option is very useful to me. I have not seen any reasoning besides "I prefer environment variables" and "the other servers don't have this option". I invite everyone to spend time developing features for Spark Connect (in particular Scala) and it will be obvious why this option is really helpful. Maybe the lack of experience in this dev-loop is driving this decision. But I stand by my opinion that improving developer efficiency and first experience should not be underestimated. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
dongjoon-hyun commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2035225318 Hi, @grundprinzip . Yes, we are debating in a healthy way. There is nothing wrong here. This is a part of healthy community process . For this part, I would not exclude @pan3793 from the user base. > nobody was complaining about it from the user base, -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
grundprinzip commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2035211798 I don't fully understand why this is an issue. It's a convenience option for users. Folks are used to run `--help` on command line scripts and see useful output. For developers in particular looking at the spark log while running the Spark Connect service is very helpful. Knowing that an environment variable exists is much more effort than calling `--help`. The part that makes it even harder for me is that this has caused no issue, no harm, nobody was complaining about it from the user base, but we debate wether or not environment variables are better than command line options. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
yaooqinn commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2034932677 Thank you for pinging me @dongjoon-hyun. Just my humble opinion. Although users can use this feature, it is primarily intended for use by the Spark Connect developers, right? It's low-maintenance, so I think we can take care of their habits. Both sys environments and command line options have their advantages. If a developer frequently needs to switch between the frontend and backend during the development process, suffixed `--wait` would be more convenient as it can be easily backspaced or auto-suggested. Well, unfortunately, `--wait` is '$1' now. How about making it more lenient about the position? -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
pan3793 commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2034776090 Additionally, `SPARK_NO_DAEMONIZE` is listed in `spark-env.sh.template` ``` ➜ conf cat spark-env.sh.template | grep SPARK_NO_DAEMONIZE # - SPARK_NO_DAEMONIZE Run the proposed command in the foreground. It will not output a PID file. ``` -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
pan3793 commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2034771042 > ... because it's not properly documented ... you can search SPARK_PREPEND_CLASSES on https://spark.apache.org/developer-tools.html > ... just use env var and update https://spark.apache.org/developer-tools.html to mention that. And I do emphasize in the PR description to update the docs to mention the env var `SPARK_NO_DAEMONIZE` usage. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
pan3793 commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2034516901 `SPARK_NO_DAEMONIZE=1 ./sbin/start-connect-server.sh` vs `./sbin/start-connect-server.sh --wait`. I don't see much benefit for the latter -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] Revert: [SPARK-47040][CONNECT] Allow Spark Connect Server Script to wait [spark]
pan3793 commented on PR #45852: URL: https://github.com/apache/spark/pull/45852#issuecomment-2034511867 cc @dongjoon-hyun @HyukjinKwon @grundprinzip @LuciferYang -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
