-
Notifications
You must be signed in to change notification settings - Fork 160
Add server console output #145
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
Conversation
| * | ||
| * @param int $length The number of lines, by default all lines will be returned. | ||
| * @return string | ||
| */ |
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 use a return typehint
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.
Thanks, done.
| * | ||
| * @param int $length The number of lines, by default all lines will be returned. | ||
| * @return string | ||
| */ |
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 the logic behind default valuelength = -1?
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.
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.
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.
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.
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.
Good point, will make that change soon 😄.
|
@casperboone Only have one small comment for you. Otherwise all looks good. |
5730866 to
a37eb93
Compare
|
@haphan Updated the implementation, could you please have another look? 😄 |
|
@casperboone Can you rebase with master branch? |
a37eb93 to
f85eb66
Compare
|
@haphan Done! :) |
This PR adds the ability to get the console output of a server (see OpenStack docs ).