Skip to content

Public terminal management - #6

Open
root-expert wants to merge 4 commits into
masterfrom
public-terminal-management
Open

root-expert wants to merge 4 commits into
masterfrom
public-terminal-management

Conversation

@root-expert

Copy link
Copy Markdown
Member

No description provided.

VarLeif added 4 commits February 24, 2020 06:53
… to add implementation with the command (shutdown, restart, etc.) and later add the authentication(?) with the jwt.
…only works if the client sends the values down, locked, up and logged_in. Also later I will have to implement the communication with the agent of the terminal for the command execution
@root-expert
root-expert requested a review from enderian March 15, 2020 15:45

@enderian enderian left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not too shabby!

Comment thread director/api/public.py
return terminal_result.serialize()

# Checks if the client sends a (not-empty-also) json
def empty_json_body(f):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Really nice, but maybe rename it to something like @require_json_body?

Comment thread director/api/public.py

# PATCH: Update only the parameters we are given
if request.method == 'PATCH':
if changes['ip'] == None: # IP is not nullable

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe run this check in the model? We should check if validations are supported by SQAlchemy.

return {"message": "Update successful"}

def update(self, changes):
madeChanges = 204

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Let's keep status codes out of the model classes, and reply inside the API classes!

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.

2 participants