Project

General

Profile

Actions
Copy link

Bug #16374

closed

[Documentation] Improve golang docs + OpenApi v3 comments

Added by Nico César over 4 years ago. Updated about 4 years ago.

Status:
Rejected
Priority:
Normal
Assigned To:
Nico César
Category:
Documentation
Target version:
2020-08-26 Sprint
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 1 (0 open — 1 closed)

Task #16543: ReviewClosedTom Clegg08/26/2020Actions

Related issues

Related to Arvados - Feature #16866: Support OpenAPINewActions
Actions
Copy link
 #1

Updated by Nico César over 4 years ago

  • Description updated (diff)
Actions
Copy link
 #2

Updated by Nico César over 4 years ago

  • Project changed from 46 to Arvados
Actions
Copy link
 #3

Updated by Nico César over 4 years ago

  • Target version set to 2020-06-03 Sprint
  • Category set to Documentation
Actions
Copy link
 #4

Updated by Peter Amstutz over 4 years ago

  • Status changed from New to In Progress
Actions
Copy link
 #5

Updated by Peter Amstutz over 4 years ago

  • Target version changed from 2020-06-03 Sprint to 2020-06-17 Sprint
Actions
Copy link
 #6

Updated by Nico César over 4 years ago

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

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

Actions
Copy link
 #7

Updated by Nico César over 4 years ago

  • Subject changed from Improve golang documentation to [Documentation] Improve golang sdk docs
Actions
Copy link
 #8

Updated by Nico César over 4 years ago

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

Actions
Copy link
 #9

Updated by Peter Amstutz over 4 years ago

  • Target version changed from 2020-07-01 Sprint to 2020-07-15
Actions
Copy link
 #10

Updated by Nico César over 4 years 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

Actions
Copy link
 #11

Updated by Nico César over 4 years 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

Actions
Copy link
 #12

Updated by Nico César over 4 years 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
Actions
Copy link
 #13

Updated by Nico César over 4 years 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

Actions
Copy link
 #15

Updated by Nico César over 4 years 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.

Actions
Copy link
 #16

Updated by Nico César over 4 years ago

  • Target version changed from 2020-08-12 Sprint to 2020-08-26 Sprint
Actions
Copy link
 #18

Updated by Nico César about 4 years ago

  • Status changed from In Progress to Rejected
Actions
Copy link
 #19

Updated by Nico César about 4 years ago

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

Actions
Copy link
 #20

Updated by Nico César about 4 years ago

Actions
Copy link

Also available in: Atom PDF