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

Added generated statik file, otherwise LCD is unusable for apps importing the SDK #2673

Closed
wants to merge 1 commit into from

Conversation

samalba
Copy link

@samalba samalba commented Nov 2, 2018

I have an app that uses the Cosmos-SDK. I need to add support for LCD, it fails to compile because the generated file statik.go file does not exist in my vendor directory. It's currently generated from the SDK Makefile, which means that all apps vendoring the SDK will miss the file.

I suggest simply adding the statik.go file in the SDK repos so any Cosmos app can enable the LCD support.

@codecov
Copy link

codecov bot commented Nov 2, 2018

Codecov Report

Merging #2673 into develop will increase coverage by 0.02%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #2673      +/-   ##
===========================================
+ Coverage     58.7%   58.72%   +0.02%     
===========================================
  Files          152      152              
  Lines         9445     9445              
===========================================
+ Hits          5545     5547       +2     
+ Misses        3530     3528       -2     
  Partials       370      370

@alexanderbez
Copy link
Contributor

alexanderbez commented Nov 3, 2018

@fedekunze Is there a reason we generate this file dynamically or part of the build process? Can we have it as a static file?

@zramsay
Copy link
Contributor

zramsay commented Nov 3, 2018

related to #2647 ... they pretty much go together afaict

@fedekunze
Copy link
Collaborator

@alexanderbez I guess we can have as a static file. Better ask @HaoyangLiu

@abelliumnt
Copy link

@fedekunze If the statik.go is static file, maybe we also need to exclude it from test_lint, besides, the init.go can be removed.

@jackzampolin
Copy link
Member

I've recently changed the way the LCD is exposed for clients to register routes. Is this PR still required?

@samalba
Copy link
Author

samalba commented Nov 29, 2018

@jackzampolin I don't remember the exact error anymore - IIRC without this you can't import this package without a panic at runtime

@jackzampolin
Copy link
Member

@samalba I'm updating our application tutorial now and will see if this is still an issue. You should check out how we are exposing the LCD to app developers now: https://github.com/cosmos/cosmos-sdk/blob/master/cmd/gaia/cmd/gaiacli/main.go#L157-L158

@samalba
Copy link
Author

samalba commented Nov 29, 2018

Sounds good, thanks @jackzampolin.

Since the new method RegisterSwaggerUI still uses statik's fs, it will miss the generated data at Runtime without this file: https://github.com/cosmos/cosmos-sdk/blob/master/cmd/gaia/cmd/gaiacli/main.go#L170

I'll simply try with the tutorial, easy to check.

EDIT: I closed the issue by mistake...

@samalba samalba closed this Nov 29, 2018
@jackzampolin
Copy link
Member

@samalba That swagger file only describes the gaia routes. Users would need to write their own swagger file to describe the routes in their app anyway. We need a way to make this more modular.

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.

6 participants