Skip to content
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

API proxy struct codegen #5854

Merged
merged 25 commits into from
Mar 23, 2021
Merged

API proxy struct codegen #5854

merged 25 commits into from
Mar 23, 2021

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented Mar 22, 2021

Includes #5844

The main part is just 300LOC in gen/api/proxygen.go, rest of the diff is moving permissions and codegen

Like docsgen, for now it's hardcoded for the api/ package, that will get fixed when porting to the versioned api

@magik6k magik6k marked this pull request as ready for review March 22, 2021 16:45
@magik6k magik6k requested a review from vyzo March 22, 2021 16:45
api/api_common.go Outdated Show resolved Hide resolved
gen/api/proxygen.go Outdated Show resolved Hide resolved
@magik6k
Copy link
Contributor Author

magik6k commented Mar 22, 2021

Ah, openrpc was getting param names from the struct, which is why those files are now smaller - diff from master:

10c10
<       "description": "```go\nfunc (c *FullNodeStruct) BeaconGetEntry(ctx context.Context, epoch abi.ChainEpoch) (*types.BeaconEntry, error) {\n\treturn c.Internal.BeaconGetEntry(ctx, epoch)\n}\n```",
---
>       "description": "```go\nfunc (s *FullNodeStruct) BeaconGetEntry(p0 context.Context, p1 abi.ChainEpoch) (*types.BeaconEntry, error) {\n\treturn s.Internal.BeaconGetEntry(p0, p1)\n}\n```",
15c15
<           "name": "epoch",
---
>           "name": "p1",
66c66
<         "url": "https://github.com/filecoin-project/lotus/blob/master/api/apistruct/struct.go#L893"
---
>         "url": "https://github.com/filecoin-project/lotus/blob/master/api/apistruct/struct.go#L885"

@magik6k magik6k requested a review from Kubuxu March 22, 2021 17:22
Copy link
Contributor

@vyzo vyzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good to me and it's definitely an improvement over having to do it by hand.

I suppose the v1 scaffolding will come at a later pr or is it planned for here?

Makefile Show resolved Hide resolved
gen/api/proxygen.go Show resolved Hide resolved
@magik6k magik6k requested a review from vyzo March 22, 2021 19:09
@Kubuxu
Copy link
Contributor

Kubuxu commented Mar 22, 2021

It was also my mistake re OpenRPC files, I through the 99% meant 99% smaller, it probably means 99% different.

Copy link
Contributor

@Kubuxu Kubuxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's squash as it doesn't build in the middle.

@magik6k magik6k merged commit c41777d into master Mar 23, 2021
@magik6k magik6k deleted the feat/apigen branch March 23, 2021 12:42
aarshkshah1992 pushed a commit that referenced this pull request Mar 26, 2021
* mostly working api proxy gen

* api: Consistent api names

* fix docsgen

* regenerate api struct

* api: expand external interfaces

* Add missing gen files

* apigen: fix perm detection

* api: Move perm tags to the interface

* gofmt

* worker perms

* docsgen

* docsgen: ignore tag comments

* apigen: add codegen warning

* gofmt

* missing actor type

* docsgen

* make linter happy

* fix lint

* apigen: use directives for tags

* docsgen

* regen openrpc docs
@magik6k magik6k mentioned this pull request Apr 13, 2021
69 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants