You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "mimikwang (via GitHub)" <gi...@apache.org> on 2025/04/12 13:11:58 UTC
[PR] feat(table): Add json parsing of table Requirement [iceberg-go]
mimikwang opened a new pull request, #393:
URL: https://github.com/apache/iceberg-go/pull/393
Added `ParseRequirement` and its string and bytes counterpart to facilitate parsing the Requirement interface based on type.
Part of #381
--
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
Re: [PR] feat(table): Add json parsing of table Requirement [iceberg-go]
Posted by "mimikwang (via GitHub)" <gi...@apache.org>.
mimikwang commented on code in PR #393:
URL: https://github.com/apache/iceberg-go/pull/393#discussion_r2040659395
##########
table/requirements.go:
##########
@@ -273,3 +279,81 @@ func (a *assertDefaultSortOrderId) Validate(meta Metadata) error {
return nil
}
+
+// ParseRequirement parses json data provided by the reader into a Requirement
+func ParseRequirement(r io.Reader) (Requirement, error) {
Review Comment:
Followed the same convention as ‘ParseMetadata’
--
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
Re: [PR] feat(table): Add json parsing of table Requirement [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #393:
URL: https://github.com/apache/iceberg-go/pull/393#issuecomment-2802557551
Not out of scope at all. Just considering that we may want to start splitting things out into more sub packages and such to reduce the size of the docs. But we can look into that as a future thing
--
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
Re: [PR] feat(table): Add json parsing of table Requirement [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #393:
URL: https://github.com/apache/iceberg-go/pull/393#issuecomment-2802256130
But we already support commiting with the rest API because you only need to marshal the requirements *to* JSON. You don't need to parse them. Unless you're trying to build a REST catalog server?
--
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
Re: [PR] feat(table): Add json parsing of table Requirement [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #393:
URL: https://github.com/apache/iceberg-go/pull/393
--
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
Re: [PR] feat(table): Add json parsing of table Requirement [iceberg-go]
Posted by "mimikwang (via GitHub)" <gi...@apache.org>.
mimikwang commented on PR #393:
URL: https://github.com/apache/iceberg-go/pull/393#issuecomment-2802248093
> This looks good, but i'm curious what the usage of this actually is
For the iceberg rest api - when you make a [post request to commit a table](https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L993), you end up with a [commit table request](https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L3408) which includes updates and requirements you have to load
--
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
Re: [PR] feat(table): Add json parsing of table Requirement [iceberg-go]
Posted by "mimikwang (via GitHub)" <gi...@apache.org>.
mimikwang commented on PR #393:
URL: https://github.com/apache/iceberg-go/pull/393#issuecomment-2802329274
> But we already support commiting with the rest API because you only need to marshal the requirements _to_ JSON. You don't need to parse them. Unless you're trying to build a REST catalog server?
yup! The rest server bit - but I understand if it's out of scope for this project - and I can implement it on our end separately - feel free to close this out
--
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
Re: [PR] feat(table): Add json parsing of table Requirement [iceberg-go]
Posted by "mimikwang (via GitHub)" <gi...@apache.org>.
mimikwang commented on code in PR #393:
URL: https://github.com/apache/iceberg-go/pull/393#discussion_r2040659395
##########
table/requirements.go:
##########
@@ -273,3 +279,81 @@ func (a *assertDefaultSortOrderId) Validate(meta Metadata) error {
return nil
}
+
+// ParseRequirement parses json data provided by the reader into a Requirement
+func ParseRequirement(r io.Reader) (Requirement, error) {
Review Comment:
Followed the same convention as ParseMetadata
--
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