You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/07/26 13:41:05 UTC
[GitHub] [iceberg] Fokko opened a new pull request, #5358: Spec: Always send the properties
Fokko opened a new pull request, #5358:
URL: https://github.com/apache/iceberg/pull/5358
The spec allows us to set the properties to `null`, but instead, we can just send the default empty dict `{}` if we don't want to set any properties at all.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on pull request #5358: Spec: Always send the properties
Posted by GitBox <gi...@apache.org>.
Fokko commented on PR #5358:
URL: https://github.com/apache/iceberg/pull/5358#issuecomment-1195745854
It is a nit here: https://github.com/apache/iceberg/pull/5287#discussion_r929473461 It is about just always sending `{}` instead of omitting the field.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #5358: Spec: Always send the properties
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5358:
URL: https://github.com/apache/iceberg/pull/5358#issuecomment-1195679547
I think we want to have as few requirements as possible, right? Is there value in doing this?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #5358: Spec: Always send the properties
Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #5358:
URL: https://github.com/apache/iceberg/pull/5358#issuecomment-1195727449
Small correction: Right now, as the field is not marked as required, implementors can skip sending the field entirely (as opposed to only having the option to send null).
We tend to do that within some of the `*Parser.toJson` functions in Java for null (e.g. not set the field).
There's nothing stopping implementors from sending the empty dict now, but the Java code at least defensively returns an empty ImmutableMap in case it parses `null`: https://github.com/apache/iceberg/blob/39878c629a6738b0fc99987ccb80da4511525bdc/core/src/main/java/org/apache/iceberg/rest/requests/CreateNamespaceRequest.java#L58-L60
Is there a benefit on the Python side to this requirement?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #5358: Spec: Always send the properties
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5358:
URL: https://github.com/apache/iceberg/pull/5358#issuecomment-1195753515
I think it makes sense for Python to not deal with `None` and to always send `{}`, but I wouldn't change the spec to require that. The spec is permissive because we expect people to make mistakes and we want service implementations to be able to accept requests that omit properties or set it to null. But we want to be as consistent as possible when producing requests. That's why I would default it in Python and always send it, but not update the spec.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on pull request #5358: Spec: Always send the properties
Posted by GitBox <gi...@apache.org>.
Fokko commented on PR #5358:
URL: https://github.com/apache/iceberg/pull/5358#issuecomment-1195756397
Got it. The reason I have in Python like that is that I've generated the code from the spec itself. If this is something that we prefer, then I think it makes sense to update the spec itself, so future generators have this as well. I don't think we need to update the Java code for example because we can always be more liberal when parsing this.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko closed pull request #5358: Spec: Always send the properties
Posted by GitBox <gi...@apache.org>.
Fokko closed pull request #5358: Spec: Always send the properties
URL: https://github.com/apache/iceberg/pull/5358
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org