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.

Sign up for GitHub

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

add api response examples to the api documentation #682

Merged
merged 2 commits into from
Sep 28, 2016
Merged

add api response examples to the api documentation #682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f00346
document examples of API responses
MaximilianMeister Sep 27, 2016
1fbe6ee
document the repositories controller
MaximilianMeister Sep 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions crowbar_framework/app/controllers/api/backups_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,36 @@ class Api::BackupsController < ApiController

api :GET, "/api/crowbar/backups", "Returns a list of available backups"
api_version "2.0"
example '
[
{
"id": 1,
"name": "testbackup",
"version": "4.0",
"size": 76815,
"created_at": "2016-09-27T06:05:10.208Z",
"updated_at": "2016-09-27T06:05:10.208Z",
"migration_level": 20160819142156
}
]
'
def index
render json: Api::Backup.all
end

api :GET, "/api/crowbar/backups/:id", "Returns a specific backup"
api_version "2.0"
example '
{
Copy link

Choose a reason for hiding this comment

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

How can we download the files?

Copy link
Member Author

Choose a reason for hiding this comment

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

with the download endpoint

"id": 1,
"name": "testbackup",
"version": "4.0",
"size": 76815,
"created_at": "2016-09-27T06:05:10.208Z",
"updated_at": "2016-09-27T06:05:10.208Z",
"migration_level": 20160819142156
}
'
def show
render json: @backup
end
Expand All @@ -35,6 +59,17 @@ def show
param :backup, Hash, desc: "Backup info", required: true do
param :name, String, desc: "Name of the backup", required: true
end
example '
{
"id": 1,
"name": "testbackup",
"version": "4.0",
"size": 76815,
"created_at": "2016-09-27T06:05:10.208Z",
"updated_at": "2016-09-27T06:05:10.208Z",
"migration_level": 20160819142156
}
Copy link

Choose a reason for hiding this comment

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

I was expecting the file to be returned as part of the backup creation.

Copy link
Member Author

Choose a reason for hiding this comment

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

why that? you mean it should be downloaded right after creation? I think that would be wrong

Copy link

Choose a reason for hiding this comment

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

Well, I guess we can use the id to download the backup file. Although I would expect an href > download > link attribute to get it from this response

'
def create
@backup = Api::Backup.new(backup_params)

Expand Down
41 changes: 41 additions & 0 deletions crowbar_framework/app/controllers/api/crowbar_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@
class Api::CrowbarController < ApiController
api :GET, "/api/crowbar", "Show the crowbar object"
api_version "2.0"
example '
{
"version": "4.0",
Copy link

Choose a reason for hiding this comment

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

Can you also document the different possible versions we'll get? What is cloud6? what's crowbar-init? what's cloud7?

Copy link
Member Author

Choose a reason for hiding this comment

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

cloud 6 is crowbar 3.0, cloud 7 is crowbar 4.0

Copy link

Choose a reason for hiding this comment

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

Please Add that as part of the documentation

Copy link
Member Author

Choose a reason for hiding this comment

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

where? as a comment on that line it would be probably too long

Copy link

Choose a reason for hiding this comment

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

I'd prefer a long line in the example than a comment on a github review ;)
You can also add those lines outside the json example and make it multiline...

I'm reading Apipie issues and it seems there is no better approach to represent the responses other than examples. A lot of people reference to swagger.io or raml.org, claiming they both support it...

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @santiph. Github PR comments should never be considered as documentation ...

Copy link
Member Author

Choose a reason for hiding this comment

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

@aspiers me too :) not sure who would not ... :/

"addons": [
"ceph",
"ha"
]
}
'
def show
render json: Api::Crowbar.status
end
Expand All @@ -28,6 +37,20 @@ def update
end

api :GET, "/api/crowbar/upgrade", "Status of Crowbar Upgrade"
example '
{
"version": "4.0",
"addons": [
"ceph",
"ha"
],
"upgrade": {
"upgrading": false, # the crowbar admin server is currently upgrading
"success": false, # the crowbar admin server has been successfully upgraded
"failed": false # the crowbar admin server failed to upgrade
}
}
'
api :POST, "/api/crowbar/upgrade", "Upgrade Crowbar"
api_version "2.0"
def upgrade
Expand All @@ -54,6 +77,24 @@ def maintenance

api :GET, "/api/crowbar/repocheck", "Sanity check for Crowbar server repositories"
api_version "2.0"
example '
Copy link
Member

Choose a reason for hiding this comment

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

Have we already agreed on the format of the result here?

Copy link
Member Author

Choose a reason for hiding this comment

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

no, but do we need to show something else besides the json response?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jsuchome @santiph as mentioned below, we may have to define some view to show extra information about response parameters that are unclear. but please in a separate small PR

{
"os": {
"available": true,
"repos": {}
},
"cloud": {
"available": false,
"repos": {
"x86_64": {
"missing": [
"SUSE OpenStack Cloud 7"
]
}
}
}
}
'
def repocheck
check = Api::Crowbar.repocheck

Expand Down
26 changes: 26 additions & 0 deletions crowbar_framework/app/controllers/api/errors_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,39 @@ class Api::ErrorsController < ApiController

api :GET, "/api/errors", "Show a list of errors"
api_version "2.0"
example '
[
{
"id": 1,
"error": "StandardError",
"message": "This is an error",
"code": 500,
"caller": "apply_role",
"backtrace": "",
"created_at": "2016-09-13T08:04:24.128Z",
"updated_at": "2016-09-13T08:04:24.128Z"
}
]
'
def index
render json: Api::Error.all
end

api :GET, "/api/errors/:id", "Show a specific error"
param :id, Integer, desc: "Error ID", required: true
api_version "2.0"
example '
{
"id": 1,
"error": "StandardError",
"message": "This is an error",
"code": null,
"caller": null,
"backtrace": "",
"created_at": "2016-09-13T08:04:24.128Z",
"updated_at": "2016-09-13T08:04:24.128Z"
}
'
def show
render json: @error
end
Expand Down
65 changes: 65 additions & 0 deletions crowbar_framework/app/controllers/api/upgrade_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,29 @@
class Api::UpgradeController < ApiController
api :GET, "/api/upgrade", "Show the Upgrade status object"
Copy link

Choose a reason for hiding this comment

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

What's the difference with upgrade under /api/crowbar/upgrade?

Copy link
Member Author

Choose a reason for hiding this comment

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

just look at the example :) /api/crowbar/upgrade doesn't include the checks

Copy link

Choose a reason for hiding this comment

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

So /api/upgrade also contains /api/crowbar/upgrade results? Could you add that as a comment in the example?

Copy link
Member Author

Choose a reason for hiding this comment

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

just ignore it for now, /api/upgrade will change this sprint anyways, as soon as we start working on https://trello.com/c/OWehKX6K/126-5-implement-a-state-machine-for-the-upgrade

Copy link

Choose a reason for hiding this comment

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

Fair enough

api_version "2.0"
example '
{
"crowbar": {
"version": "4.0",
"addons": [
"ceph",
"ha"
],
"upgrade": {
"upgrading": false,
"success": false,
"failed": false
}
},
"checks": {
"sanity_checks": true,
"maintenance_updates_installed": true,
"clusters_healthy": true,
"compute_resources_available": true,
"ceph_healthy": true
}
}
'
def show
render json: Api::Upgrade.status
end
Expand Down Expand Up @@ -64,6 +87,15 @@ def services

api :GET, "/api/upgrade/prechecks", "Shows a sanity check in preparation for the upgrade"
api_version "2.0"
example '
{
"sanity_checks": true,
"maintenance_updates_installed": true,
"clusters_healthy": true,
"compute_resources_available": true,
"ceph_healthy": true
}
'
def prechecks
render json: Api::Upgrade.check
end
Expand All @@ -82,6 +114,39 @@ def cancel

api :GET, "/api/upgrade/repocheck", "Check for missing node repositories"
api_version "2.0"
example '
{
"ceph": {
"available": false,
"repos": {}
},
"ha": {
"available": false,
"repos": {}
},
"os": {
"available": true,
"repos": {}
},
"openstack": {
"available": false,
"repos": {
"missing": {
"x86_64": [
"SUSE-OpenStack-Cloud-7-Pool",
"SUSE-OpenStack-Cloud-7-Updates"
]
},
"inactive": {
"x86_64": [
"SUSE-OpenStack-Cloud-7-Pool",
"SUSE-OpenStack-Cloud-7-Updates"
]
}
}
}
}
'
def repocheck
render json: Api::Upgrade.repocheck
end
Expand Down
Loading