Skip to content

Add server console output - #145

Merged
haphan merged 3 commits into
php-opencloud:masterfrom
casperboone:console_output
Dec 19, 2017
Merged

Add server console output#145
haphan merged 3 commits into
php-opencloud:masterfrom
casperboone:console_output

Conversation

@casperboone

@casperboonecasperboone commented Aug 28, 2017

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
ContributorAuthor

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
Copy Markdown
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
Copy Markdown
ContributorAuthor

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
Copy Markdown
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
Copy Markdown
ContributorAuthor

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
Copy Markdown
Collaborator

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

@casperboone

Copy link
Copy Markdown
ContributorAuthor

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

@haphan

Copy link
Copy Markdown
Collaborator

@casperboone Can you rebase with master branch?

@casperboone

Copy link
Copy Markdown
ContributorAuthor

@haphan Done! :)

@haphan
haphan merged commit 328705b into php-opencloud:masterDec 19, 2017
Sign up for freeto 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

@casperboone@haphan@laszlof