Skip to content

Conversation

@casperboone
Copy link
Contributor

@casperboone casperboone commented Aug 28, 2017

This PR adds the ability to get the console output of a server (see OpenStack docs ).

*
* @param int $length The number of lines, by default all lines will be returned.
* @return string
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use a return typehint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done.

*
* @param int $length The number of lines, by default all lines will be returned.
* @return string
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the logic behind default valuelength = -1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When given a length of -1, the API will return all lines. Since the default (when no length is specified) in the OpenStack docs is returning all lines as well, I thought it was reasonable to make this the default here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

When given a length of -1, the API will return all lines

This is an assumption thus subject to change in the future; I think we better follow official docs that length shall not be included in the request if we want API to return all lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, will make that change soon 😄.

@haphan
Copy link
Collaborator

haphan commented Sep 3, 2017

@casperboone Only have one small comment for you. Otherwise all looks good.

@casperboone
Copy link
Contributor Author

@haphan Updated the implementation, could you please have another look? 😄

@haphan
Copy link
Collaborator

haphan commented Nov 9, 2017

@casperboone Can you rebase with master branch?

@casperboone
Copy link
Contributor Author

@haphan Done! :)

@haphan haphan merged commit 328705b into php-opencloud:master Dec 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants