-
Notifications
You must be signed in to change notification settings - Fork 16
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
Docs for list view and cluster import #65
Docs for list view and cluster import #65
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I have only few comments.
---------- | ||
curl -XPOST -H "Content-Type: application/json" \ | ||
-d '{ "node_ids": ["7026f371-2d8c-4ad7-895f-4d4dcc70ff1c"], \ | ||
"sds_type":"ceph"}' http://127.0.0.1/api/1.0/ImportCluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add Gluster related example or mention what is a difference between importing cluster for Gluster and Ceph?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that depends on the sds_type you send, it could be ceph or gluster.
---------- | ||
Status: 200 OK | ||
[{ | ||
"integration_id": "717315a1-2a49-4cb7-826b-d2aa3bc4721a", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be maybe specified job_id in a response so it would be easier to access one item from jobs list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The job_id will be in response to ImportCluster for ex. which can be used the query the job with the Jobs Details API
Sample Request | ||
---------- | ||
curl -XPOST -d '{"Volume.volname":"Volume_009","Volume.bricks":["dhcp-1.lab.ten | ||
drl.example:/root/bricks/vol9_b1"]}' http://127.0.0.1/api/1.0/5291c055-70d3-4450-9769-2f6fd4932afb/GlusterCreateVolume |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be better to add example with more bricks and show how to specify how to create replicated/striped volumes if they are supported. Volume with one brick is a little misleading.
"status": "finished", | ||
"flow": "ImportCluster", | ||
"parameters": { | ||
"integration_id": "717315a1-2a49-4cb7-826b-d2aa3bc4721a", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there some way how to get cluster_id from job?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fbalak the integration_id is the cluster_id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I also overlooked that there is cluster_id in parameters section of response (on line 37). Just why is it different from integration_id if they are same?
|
||
== Import Cluster | ||
|
||
Import an existing Gluster or Ceph cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think documentation and description of arguments will help to use this function. For example where user can get node IDs which can use in this function; where users can get list of available storage types(function or static list in documentation.
This comment is valid for all functions. Please describe function arguments.
---------- | ||
Status: 202 Accepted | ||
|
||
{ job_id: "3784922e33e8bec939be5e626e21a174" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add to docs where user can use this ID in return message..
|
||
---------- | ||
curl -XGET | ||
http://127.0.0.1/api/1.0/jobs/1cb0694c-8041-4576-95ee-f01f07738ff9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is 1cb0694c-8041-4576-95ee-f01f07738ff9 and where can user get it? I understand that this is trivial question but answer should be written in docs.
|
||
`DELETE` to delete a resource | ||
|
||
=== Authentication |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this accordingly with Tendrl/specifications#128 ?
`Authorization: Tendrl <API_TOKEN>`. | ||
|
||
---------- | ||
curl -H "Authorization: Tendrl <API_TOKEN>" http://127.0.0.1/api/1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where user get API_TOKEN?
3) Sending invalid fields will result in a `422 Unprocessable Entity` | ||
response. | ||
|
||
=== HTTP Verbs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can these verbs be applied to list of resources and single resources? See https://en.wikipedia.org/wiki/Representational_state_transfer#Relationship_between_URL_and_HTTP_methods
|
||
=== User Agent | ||
|
||
Every API request must send a `User-Agent` property in the header to identity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this anyhow related to API_TOKEN? If yes, how?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After conversation with @anivargi ; approving this PR and getting it ready for merge.
@sankarshanmukhopadhyay Is it possible to comment my questions, please? Or should I file documentation issues because this pull request is merged now? |
No description provided.