Skip to content
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

Public terminal management #6

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

root-expert
Copy link
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
Copy link

@enderian enderian left a comment

Choose a reason for hiding this comment

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

Not too shabby!

return terminal_result.serialize()

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

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?


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

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

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