[GitHub] [spark] HyukjinKwon commented on pull request #30167: [SPARK-33240][SQL][3.0] Fail fast when fails to instantiate configured v2 session catalog
HyukjinKwon commented on pull request #30167: URL: https://github.com/apache/spark/pull/30167#issuecomment-718336795 _In a way_, someone could think it's all internal as long as the tables are created somewhere and they can be read regardless of where the tables are created. And, someone _could_ think "there's no functional 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] HyukjinKwon commented on pull request #30167: [SPARK-33240][SQL][3.0] Fail fast when fails to instantiate configured v2 session catalog
HyukjinKwon commented on pull request #30167: URL: https://github.com/apache/spark/pull/30167#issuecomment-718295109 @HeartSaVioR, I fully agree that the newer behaviour makes much sense now. I also get that we can have many benefits by doing this fix. We have many fixes that benefit a lot in master branch. As you pointed out, my point is that the fallback behaviour _might_ make sense in few contexts, e.g. https://github.com/apache/spark/pull/30167#issuecomment-717897756 and I wouldn't expect such behaviour changes in a maintenance release. This makes me less sure if we should port back or not. 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] HyukjinKwon commented on pull request #30167: [SPARK-33240][SQL][3.0] Fail fast when fails to instantiate configured v2 session catalog
HyukjinKwon commented on pull request #30167: URL: https://github.com/apache/spark/pull/30167#issuecomment-717982630 I am not sure. It looks to me less usual to port back a fix for maintenance release when there's a possibility that breaks something that can make sense. I would be fine if other committers prefer it. 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] HyukjinKwon commented on pull request #30167: [SPARK-33240][SQL][3.0] Fail fast when fails to instantiate configured v2 session catalog
HyukjinKwon commented on pull request #30167: URL: https://github.com/apache/spark/pull/30167#issuecomment-717897756 @RussellSpitzer, I completely agree with this fix itself. But about porting it back: The flip side works too. For example, for any reason, in a cluster the catalog was set and users used to use it with no problem. User don't want to change their existing workload and code but switch the catalog they are using. So they just reset the configuration from their cluster. After the fix, this case wouldn't work anymore. Of course, I like this failfast way so it can prevent such misuse but from user's perspective I'd imagine they would think a behaviour change which should ideally not happen in maintenance release. To be more clear, - if users set their catalog properly, things work correctly with/without this fix. - this fix throws an exception for the case (we defined as misuse now) which might have worked before I think it's arguable to call it a bug fix to port back. 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] HyukjinKwon commented on pull request #30167: [SPARK-33240][SQL][3.0] Fail fast when fails to instantiate configured v2 session catalog
HyukjinKwon commented on pull request #30167: URL: https://github.com/apache/spark/pull/30167#issuecomment-717731021 All bug fixes bring behavioural changes. I think the point is if that previous behaviour was intended or not when it was proposed and merged. The previous behaviour looks intended with a proper error message (which I agree wasn't a good call). Sure, waiting other people' opinions are fine. 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] HyukjinKwon commented on pull request #30167: [SPARK-33240][SQL][3.0] Fail fast when fails to instantiate configured v2 session catalog
HyukjinKwon commented on pull request #30167: URL: https://github.com/apache/spark/pull/30167#issuecomment-717719946 We changed a behaviour after discussion, and I think It's just a design call we newly made. The previous behaviour also make sense by itself - falling back although it was a bad design decision. 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] HyukjinKwon commented on pull request #30167: [SPARK-33240][SQL][3.0] Fail fast when fails to instantiate configured v2 session catalog
HyukjinKwon commented on pull request #30167: URL: https://github.com/apache/spark/pull/30167#issuecomment-717706506 Hm, is it something we should port back? I think it's hard to call it a bug. Maintenance release shouldn't have such behaviour changes in general according to semver though I got that DSv2 is still experimental. 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
