Bug #16374

[Documentation] Improve golang docs + OpenApi v3 comments

Added by Nico César 9 months ago. Updated 5 months ago.

Status:
Rejected
Priority:
Normal
Assigned To:
Nico César
Category:
Documentation
Target version:
2020-08-26 Sprint
Start date:
08/26/2020
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-

Description

https://doc.arvados.org/v1.4/sdk/go/example.html has the title "Arvados | Python | Examples"

the example starts with a #<Liquid::Comment:0x0000561e37195428> that should not be there.

and compilation fails:

go build .
go: finding module for package git.curoverse.com/arvados.git/sdk/go/arvadosclient
go: downloading git.curoverse.com/arvados.git v0.0.0-20200424144959-9e9775694f6b
go: found git.curoverse.com/arvados.git/sdk/go/arvadosclient in git.curoverse.com/arvados.git v0.0.0-20200424144959-9e9775694f6b
go: github.com/nicocesar/arvados-forecaster imports
    git.curoverse.com/arvados.git/sdk/go/arvadosclient: git.curoverse.com/arvados.git@v0.0.0-20200424144959-9e9775694f6b: parsing go.mod:
    module declares its path as: git.arvados.org/arvados.git
            but was required as: git.curoverse.com/arvados.git

s/git.curoverse.com/git.arvados.org/g did make it work BTW

also golint output:

golint main.go 
main.go:37:17: struct field Uuid should be UUID
main.go:75:9: don't use underscores in Go names; var collection_fields_wanted should be collectionFieldsWanted
main.go:102:25: don't use underscores in Go names; var item_map should be itemMap

Subtasks

Task #16543: ReviewClosedTom Clegg

Related issues

Related to Arvados - Feature #16866: Support OpenAPINew

Associated revisions

Revision 22c4e045 (diff)
Added by Nico Cesar 6 months ago

WIP added minimum boilerplate to test OpenAPI v3 support

refs #16374

Arvados-DCO-1.1-Signed-off-by: Nico Cesar <>

Revision b3eed876 (diff)
Added by Nico Cesar 6 months ago

WIP added minimum boilerplate to test OpenAPI v3 support

refs #16374

Arvados-DCO-1.1-Signed-off-by: Nico Cesar <>

History

#1 Updated by Nico César 9 months ago

  • Description updated (diff)

#2 Updated by Nico César 9 months ago

  • Project changed from Lightning private to Arvados

#3 Updated by Nico César 8 months ago

  • Target version set to 2020-06-03 Sprint
  • Category set to Documentation

#4 Updated by Peter Amstutz 8 months ago

  • Status changed from New to In Progress

#5 Updated by Peter Amstutz 8 months ago

  • Target version changed from 2020-06-03 Sprint to 2020-06-17 Sprint

#6 Updated by Nico César 7 months ago

  • Target version changed from 2020-06-17 Sprint to 2020-07-01 Sprint

new branch 16374-improve-go-doc (wip)

#7 Updated by Nico César 7 months ago

  • Subject changed from Improve golang documentation to [Documentation] Improve golang sdk docs

#8 Updated by Nico César 7 months ago

started branch 16374-godoc-improvements to add comments that will reflect in godoc

#9 Updated by Peter Amstutz 7 months ago

  • Target version changed from 2020-07-01 Sprint to 2020-07-15

#10 Updated by Nico César 6 months ago

  • Target version changed from 2020-07-15 to 2020-08-12 Sprint

Maybe we should close this one for now, there is a branch but doesn't have much, and is mostly about comments in the code

#11 Updated by Nico César 6 months ago

  • Subject changed from [Documentation] Improve golang sdk docs to [Documentation] Improve golang docs + OpenApi v3 comments

Today during the engineering meeting we talk about starting OpenAPI v3 comments in some of the controller methods so they can be used as documentation and maybe in the future to implement clients in different language

Following https://github.com/mikunalpha/goas I started with a prototype in the 16534-example-api-pkg branch

#12 Updated by Nico César 6 months ago

commit 22c4e0450 is just a test

(I had to change go.mod because of https://github.com/mikunalpha/goas/issues/10 hoping that they fix that soon)

to install https://github.com/mikunalpha/goas:

go get github.com/mikunalpha/goas


cd ~/arvados
~/go/bin/goas --debug --module-path . --main-file-path cmd/arvados-server/cmd.go

that will generate oas.json

to generate a client for example:

docker run -e GO_POST_PROCESS_FILE="/usr/local/bin/gofmt -w" --rm -v ${PWD}:/local openapitools/openapi-generator-cli generate     -i  /local/oas.json    -g go     -o /local/out/go

#13 Updated by Nico César 6 months ago

after a long session of debugging I concluded that https://github.com/mikunalpha/goas/issues/10 was a bigger problem than I thought so I migrated to go standard library and submited a PR https://github.com/mikunalpha/goas/issues/11

I don't think the author is actively developing this, So I created my own fork. We'll see if this ends up being a good idea or not, I'm also gathering information on the rationale of the rest of the forks

#15 Updated by Nico César 6 months ago

I tried to use the https://github.com/LucyBot-Inc/api-spec-converter unsuccessfully

wget -O test.json https://pirca.arvadosapi.com/discovery/v1/apis/arvados/v1/rest
grep -E '"type":"[^"]+"' test.json  --color -o|sort -u
"type":"array" 
"type":"Array" 
"type":"boolean" 
"type":"datetime" 
"type":"float" 
"type":"Hash" 
"type":"integer" 
"type":"object" 
"type":"string" 
"type":"text" 

[nico:~] $ api-spec-converter -f google -t openapi_3 test.json
Fatal AssertionError: Error during conversion: Error during conversion: type specified not supported
    at processParameter (/usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/google-discovery-to-swagger/src/index.js:349:10)
    at /usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/google-discovery-to-swagger/src/index.js:333:19
    at /usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/lodash/lodash.js:4925:15
    at baseForOwn (/usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/lodash/lodash.js:2990:24)
    at /usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/lodash/lodash.js:4894:18
    at Function.forEach (/usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/lodash/lodash.js:9368:14)
    at LodashWrapper.object.(anonymous function) [as each] (/usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/lodash/lodash.js:15758:25)
    at processParameterList (/usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/google-discovery-to-swagger/src/index.js:332:34)
    at processMethod (/usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/google-discovery-to-swagger/src/index.js:281:22)
    at processMethodList (/usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/google-discovery-to-swagger/src/index.js:229:33)
    at processSubResource (/usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/google-discovery-to-swagger/src/index.js:235:17)
    at /usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/google-discovery-to-swagger/src/index.js:182:24
    at /usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/lodash/lodash.js:4925:15
    at baseForOwn (/usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/lodash/lodash.js:2990:24)
    at /usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/lodash/lodash.js:4894:18
    at Function.forEach (/usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/lodash/lodash.js:9368:14)

As we can see in
/usr/local/node-v6.11.2-linux-x64/lib/node_modules/api-spec-converter/node_modules/google-discovery-to-swagger/src/index.js line 349

  assert.ok(['string', 'number', 'integer', 'boolean'].indexOf(param.type) >= 0, 'type specified not supported');

.... only this 4 types are accepted.

#16 Updated by Nico César 6 months ago

  • Target version changed from 2020-08-12 Sprint to 2020-08-26 Sprint

#18 Updated by Nico César 5 months ago

  • Status changed from In Progress to Rejected

#19 Updated by Nico César 5 months ago

We dont have the bandwith to do all the OpenApi v3 specification right now, I'm closing the ticket for now

#20 Updated by Nico César 3 months ago

Also available in: Atom PDF