ga4gh / task-execution-schemas

Apache License 2.0
80 stars 28 forks source link

Generate swagger json from .proto using CI #97

Closed psafont closed 5 years ago

psafont commented 6 years ago

Hi, I saw that the swagger json is not kept up to date, so I thought it would be a good idea to keep it up-to-date using CI.

I've taken ideas from GA4GH server repo to generate the json using travis CI and uploading it to the Github releases page.

This means that commits need to be tagged for the definition to be generated, but it reduces the amount of handiwork needed to maintain it up to date.

The encrypted API token should work as it is mine, but I think it should be changed to one from a ga4gh helper account.

Any thoughts?

geoffjentry commented 6 years ago

I like the idea!

Is it really the right thing to be doing to jam that key in the public repo?

buchanae commented 6 years ago

I believe that key gives write access to repositories via your user. Probably want to revoke that token.

david4096 commented 6 years ago

I'm +1 for this change. The only loss is having a nice link to the swagger in the root of the repo. Suggest hard linking to the most recent release's swagger from the README.md.

This is the right way, but should be replaced by someone's key that has rights to make releases in TES.

fwiw in DOS we wrap protoc in cwltool. Requiring go and protoc is a pretty tall order and most of these developers probably have Docker.

buchanae commented 6 years ago

I think there's a latest alias for github releases. Might be useful.

Ah, ok, I guess that's an encrypted key? I'm reading this: https://docs.travis-ci.com/user/deployment/releases/#Authenticating-with-an-OAuth-token

I'd like to avoid a dependency on cwltool please. If go + protoc is a problem, a single docker container could be provided.

psafont commented 6 years ago

Yes, the key is encrypted :)

I mentioned a Github helper account to so nobody has to even an encrypted token with write permissions, it would mitigate possible problems if somehow a breach happened.

@buchanae you're right, we can always link to the latest release in the readme: https://github.com/psafont/task-execution-schemas/releases/latest

The problem is that the artifacts of the release cannot be linked this way. For example, this doesn't work: https://github.com/psafont/task-execution-schemas/releases/download/latest/task_execution.swagger.json

buchanae commented 6 years ago

The problem is that the artifacts of the release cannot be linked this way. For example, this doesn't work: https://github.com/psafont/task-execution-schemas/releases/download/latest/task_execution.swagger.json

Ah, bummer!

If auto-building the swagger doesn't work out, another option is to force developers to build it by having Travis fail the build if the generated swagger differs from the committed version.

Yet another option is to maybe publish a gh-pages branch with the latest swagger and documentation.

psafont commented 6 years ago

Should the linking / presentation of the files be in a separate issue? I'm testing what can I do with github pages anyway, never used them before: https://psafont.github.io/task-execution-schemas/

buchanae commented 6 years ago

Should the linking / presentation of the files be in a separate issue?

Ya, that's fine.

david4096 commented 6 years ago

On a related note since you mentioned github pages @psafont, it would be awesome to have a static site generated with swagger editor/swagger UI to go along with our sites instead of using their hosted editors.

https://github.com/ga4gh/data-object-schemas/issues/26

psafont commented 6 years ago

I did a try using spectacle: https://cdn.rawgit.com/psafont/task-execution-schemas/docgen/docs/index.html

The problem is that the repo itself needs to be changed again to include the documentation, having it in a separate one might be better.

psafont commented 6 years ago

I rebased the branch.

Aside from the helper github account for uploading the release and adding the corresponding key, are there any other holdups for the merge?

Maybe it could even be done without the helper account.

buchanae commented 6 years ago

I think it's just the helper account. @kellrott could you advise?

kellrott commented 6 years ago

I don't know if there is any policy about this. And I don't think I have the access rights to setup the keys. @briandoconnor have any of the other GA4GH projects setup this kind of linkage to a build system before?

delagoya commented 5 years ago

Can we close this issue since Protobuf is being deprecated?

susheel commented 5 years ago

+1