amogh-jahagirdar commented on code in PR #9695: URL: https://github.com/apache/iceberg/pull/9695#discussion_r1690318781
########## open-api/rest-catalog-open-api.yaml: ########## @@ -537,6 +537,113 @@ paths: 5XX: $ref: '#/components/responses/ServerErrorResponse' + /v1/{prefix}/namespaces/{namespace}/tables/{table}/preplan: Review Comment: > In other words, it would make no sense for preplan to return 501 because it is just as easy to fake a plan task and have the plan endpoint do all the real work. This is a good point, I wasn't really considering it from this angle but yeah it's not much of a lift to just return an opaque plan task where the actual plan endpoint does the work. this is one example in favor of not having to introduce another capability. > In scenario 1, the client calls plan and the service can decide whether to redirect it to preplan using 521. @rdblue I think you mean 422? https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/422 but agree with the idea Overall the approach of working from how plan/preplan will be called first or not seems reasonable, at least it establishes what a sane default would be for the average case (calling plan table first). I'm not sure I'd entirely base the decision on capabilities based off of this since as @Fokko said different clients can do different things depending on their use case but the first point around "it's as easy just to return an opaque token as it is to throw a 501" convinced me that it's not worth having a separate capability. I think that's actually a good heuristic for determining if something should be a new capability; is the lift heavy to have a simple implementation compared to throwing a 501 or not? In this case it's essentially a no-op implementation so the complexity of having a different capability probably isn't worth it. >I'm not sure if we really should force the client into certain directions. @Fokko so I actually don't know if we're forcing clients, more so establishing what a sane default is for which endpoint should be called first. This default would be established off the average case, which I think I agree with @rdblue that most cases plan endpoint would just work without redirection. If a client has to preplan in it's average case workloads then I think they can still do that, maybe there's a client configuration to enable that but that's really in the implementation I'd say. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org