Skip to content

Jonathan Zhang - Ground Station onboarding#69

Open
JonathanmZhang wants to merge 3 commits intoUWOrbital:mainfrom
JonathanmZhang:main
Open

Jonathan Zhang - Ground Station onboarding#69
JonathanmZhang wants to merge 3 commits intoUWOrbital:mainfrom
JonathanmZhang:main

Conversation

@JonathanmZhang
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)
logger_middleware ss
  • I am interested to work on the frontend (optional)

New Changes

  • Completed the backend onboarding tasks, which includes creating a create and delete command, a validating params format function and a method that logs any requests.

Testing

  • I tested using pytest . to see if there were any errors that would popup, and I ran the backend server to check what log data would actually be outputted, and if that matches the custom logging module.

Copy link

@proprogrammer504 proprogrammer504 left a comment

Choose a reason for hiding this comment

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

Looks good overall! Just a few very minor changes.

request_time = to_unix_time(datetime.datetime.now())

response: Response = await call_next(request)
duration = time.perf_counter() - start

Choose a reason for hiding this comment

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

Minor thing, any way to convert this into ms?

if (self.params != None and self.format != None) and len(self.params.split(",")) == len(self.format.split(",")):
return self
else:
raise ValueError("Value error")

Choose a reason for hiding this comment

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

Could you add a more descriptive error message?


command = db.exec(select(Command).where(Command.id == id)).one_or_none()
if command is None:
raise HTTPException(status_code=404, detail="Error")

Choose a reason for hiding this comment

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

A more detailed error message would be nice.

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