[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-727149697 I just initiated the discussion on dev@ mailing list which I should have been done instead. https://lists.apache.org/thread.html/r30069e17f59e8d29267ae296d56840970905476019023f20164ee5a3%40%3Cdev.spark.apache.org%3E My apologize to make noises and feel anyone unhappy. 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. 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
[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-727149286 Sorry to all for all noises. Please disregard all conversation. I'll remove them now. 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. 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
[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-727138220 I was feeling odd and became feeling upset because my intention wasn't change from the first comment and the intention was disregarded (at least that's what I felt like) for a couple of early replies, even I emphasized whether you checked my comments were addressed or not is not the point of concern here. The replies were simply based on whether there was any fault or not, and in addition, trying to interpret my action on your view and framed me. I never said my approval is required because I'm domain expert of SS. I mentioned twos including me because twos are only the reviewers who left "actual" review comments, not saying we're dealing with SS hence we are gatekeeper. This is unchanged from the first comment and I never said about SS in my comments. Same happened for the statement I'm framing the Apache Spark community be the negative. I'm going to be a bad guy based on what I even didn't mention. How I can't be upset? Can we make sure we carefully read the comment and try not to interpret and just ask if we're unsure about the intention? Other than that I feel this is a waste of time. I have been trying to explain the committer's power to say "technically correct" is not always representing "we are doing good", but I also admit it's super hard to reconsider the practice we have been using for a while, and I feel the air that we're reluctant to even think about it. At least I shouldn't deal with individual, as it's against my voice as well and that feels someone to be educated. My apologize again. 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. 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
[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-727121487 And I also admit I have different voice on post-review. I agree post-review would open the possibility for reviewers to review later who weren't active during the review period before merging, but IMHO it's over-use of post-review if it's being used to provide merger rationalization to cut the review period at any time, feel other reviewers to be **disregarded**. 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. 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
[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-727120756 I was writing a wall of text and Chrome happily (?) killed itself. Rewriting one. What I really asked you to do is exactly this. The practice is also happened for the first time I made a first contribution on Apache Spark. If you want I can find that too. https://github.com/apache/spark/pull/30139#issuecomment-727103124 I'm really unhappy now you're saying I'm framing Apache Spark community to be negative ways. I was never saying about the community here. Let me quote my comment again, where's the strong voice like the attacking or making things being negative, and mentioning any sort of thing representing community? > (I hope the PR can wait for reviewers' approvals who left valid review comments. I understand the PR can be merged "technically", but doesn't seem to be a good practice. If in doubt w.r.t reviewers being inactive, we can at least wait for a day based on timezone difference.) This comment is exactly explaining the same thing what I link for reference. Could you find any difference? Even I didn't say that strongly. A PMC member is just a "one of" PMC members. Before you make consensus along with PMC members and construct a BYLAWS, you can't frame me as one guy complaining to Spark community. The practice is not a thing "everyone" has a belief, so attacking the practice shouldn't be interpreted like so. Makes sense? For sure, I said about -1 as a sort of joke, but what you requested to me to block merger to merge at any time is "technically" the same. That is just a "representational" difference, and for me it's also not welcomed one if I have to always mention merger to stop doing before I approve. And this suggestion is your own, not from the consensus. But in overall, I have to admit I really did one bad thing, trying to educate someone which is not constructed like BYLAW in any way, regardless of the intention. I should avoid doing that and raise discussion in dev mailing thread like I have been doing before. My apologize. 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. 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
[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-726569827 I'd rather avoid the chance of "post-review" whenever possible, but I'd admit everyone has different thoughts. I'm OK with it, and if that's considered here (and no one would be disappointed from post-review), I was clearly wrong. My bad. I hope we prefer to say about "good practice" of doing something instead of saying "whether someone is the right to do something". Everything is technically possible. e.g. I don't think Spark has some sort of BYLAWS which prevents committers to self +1 and push, but I don't leverage it even I still have PRs sitting around years which got LGTM from contributors. It's just because it's a bad practice. Agree to disagree about the different understanding of "details" about good practice of reviewing is OK (because I may be wrong), but I'd be unhappy if we keep saying about the "right". This says I should always start with explicitly vote -1 whenever I start reviewing, which is the only way about the claim of "my right" of reviewer, but no one would do this as we have a belief and huge negative effect on putting veto. And sure I don't change my reviewing practice to start from -1. (That's gonna be just funny.) 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. 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
[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-726433238 What do you say? I don't say you're missing the comment. I say the confirmation is better to be done by the reviewers who review, not from some other one. The confirmation would only take a day or so (due to the timezone) if the reviewers are quite active. Even you can say "I'll merge in tomorrow if there's no further feedback." to force the delay to a day. You're emphasizing 2 weeks but I can enumerate the PRs which waits for a half of year or even years. The code change is quite small (because the detection logic was there), but given it's breaking backward compatibility, it's not odd request to make a consensus. If the default is false probably we rather concern less. 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. 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
[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-726428939 No that's not also my point. I don't claim about the domain owner (but in practice I see there's implicit domain owner). You can merge the PR in SS area without me if I didn't review. The concern is that I was the one of active reviewers and you're just assuming my review comments were addressed without my confirmation. Both my and @xuanyuanking's comments. I don't require others to read my mind, and anyone can't read my mind, so there should be the process of "confirmation". That would just bring delay of merging a day or so, wouldn't hurt anything. 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. 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
[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-726418896 There's no indication I have produced all review comments (while actually I produced all review comments), and review comment author would be the right one to verify whether the review comments are addressed as suggested. I planned to review the new change again today but it's already merged. I see we tend to ask reviewers to see whether they're happy with the last change before merging. At least we "were", and I tend to ensure that, unless reviewers are not responded for days. I'd hope we ensure everyone is happy with the change before merging, otherwise discuss/debate if necessary. 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. 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
[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-726410128 (I hope the PR can wait for reviewers' approvals who left valid review comments. I understand the PR can be merged "technically", but doesn't seem to be a good practice. If in doubt w.r.t reviewers being inactive, we can at least wait for a day based on timezone difference.) 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. 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
[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-725867167 > Let's also mention the behavior change in ss-migration-guide.md Let's make sure this review comment is also addressed as well. I just skipped mentioning it as it's already commented. 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. 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
[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-721373759 That's the chicken and egg problem, you know. dev@ list is not so active because all the important discussions aren't passing through the dev list, which is I think bad in perspective of "community over code". 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. 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
[GitHub] [spark] HeartSaVioR commented on pull request #30210: [SPARK-33259][SS] Disable streaming query with possible correctness issue by default
HeartSaVioR commented on pull request #30210: URL: https://github.com/apache/spark/pull/30210#issuecomment-720876379 No I don't have real case for knowing and taking the risk. Probably I could create some query which could evade the issue, but I agree that's more likely in theory and not real case. Saying again I don't object the change. If you look back my proposal then you'll find blocking the query is also one of options in my proposal. My point was that such change warrants the discussion, ideally in dev@ mailing list instead of PR. 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. 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
