-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: enable querying bucket, object and group by id. #83
Conversation
Use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add some API that match the NFT metadata standard.
x/storage/keeper/query.go
Outdated
if !found { | ||
return nil, types.ErrNoSuchObject | ||
} | ||
attributes := make([]types.Trait, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we have a generic function to convert struct to NFT metadata standard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also it's better to add an interface (e.g ToNFTMetadata
) to bucket/object/group info struct.
docs/static/swagger.yaml
Outdated
@@ -1327,6 +1327,117 @@ paths: | |||
type: string | |||
tags: | |||
- Query | |||
/bnb-chain/greenfield/sp/storage_provider/{spAddress}: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will sp/storage_provider
such a path be verbose? since sp
is already short for storage_provider
.
@fynnss @alexgao001 what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is auto-generated by ignite. sp means the sp module.
I think we can make this more simple by remove the sp
.
type: string | ||
format: byte | ||
parameters: | ||
- name: spAddress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the name spAddress
is too generic, which address is it exactly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same as the validatorAddress that we use the operator address of it as the ValAddress.
Any other advice?
the object is public, everyone can access it. | ||
content_type: | ||
type: string | ||
description: >- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: make it an enum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is generally represented by a string for content-type in minio or ceph or s3
create_at: | ||
type: string | ||
format: int64 | ||
title: create_at define the block number when the object created |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you double check all create_at
? I think some are not clear, we may think it is a unix timestamp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we can rename it to create_height
.
It will be associated with some system parameters, such as redundant parameters.
'200': | ||
description: A successful response. | ||
schema: | ||
type: object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swagger
is expected to define the object and reuse it. I think we redefine the bucket, storage object in many places, any idea to improve that?
…enfield into feat-storage-indexed-by-id
Description
Rationale
tell us why we need these changes...
Example
Changes
Notable changes: