Skip to content

Kai ZHang GS-Onboarding#39

Open
Scr4tch587 wants to merge 5 commits intoUWOrbital:mainfrom
Scr4tch587:main
Open

Kai ZHang GS-Onboarding#39
Scr4tch587 wants to merge 5 commits intoUWOrbital:mainfrom
Scr4tch587:main

Conversation

@Scr4tch587
Copy link

Purpose

Completed the GS on-boarding task. Include a screenshot of the front-end of the application.

  • I have added my name to the onboarding title (required)
  • I have added a screenshot of the logs printed by the logger middleware (required)
  • I am interested to work on the frontend (optional)

New Changes

Backend

  • Added data validation for command requests
  • Added a post endpoint for commands, allowing for them to be added to the database
  • Added a delete endpoint for commands, allowing for them to be removed from the database
  • Added a logger middleware
    Frontend
  • Added the functionality to delete commands and linked it to the delete endpoint of the backend using Axios
  • Created a selection form to allow users to create new commands based on MainCommands

Testing

  • Explain tests that you ran to verify code functionality.
  • Passed all pytests
image - Checked the state of the gs_python.log printed by the logger middleware image - Locally ran the frontend and tested the functionality of adding/deleting commands image

Copy link

@Adityya-K Adityya-K left a comment

Choose a reason for hiding this comment

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

I have reviewed the backend, another lead will review the front end! Great first push, just some changes with error logging, unnecessary overhead and reused functionality

else:
db.delete(item_to_delete)
db.commit()
query = select(Command)

Choose a reason for hiding this comment

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

This seems like repeated functionality, you can use the get_commands() function here!

end_time = time.perf_counter()
execution_time = end_time - start_time

logger_setup()

Choose a reason for hiding this comment

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

We shouldn't be calling these functions on every dispatch since we are just starting and opening the logger over and over again which is unnecessary overhead.

"""
# TODO: (Member) Implement this method
num_values = 0
if self.params != None:

Choose a reason for hiding this comment

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

Try making these ValueError more descriptive by adding a message string within brackets ValueError("Something went wrong") (of course, make the message descriptive)

Copy link

@Adityya-K Adityya-K left a comment

Choose a reason for hiding this comment

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

LGTM! Not closing, in case the frontend is reviewed.

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