Skip to content

After Feedback version#20

Closed
19prishpat wants to merge 5 commits intoUWOrbital:mainfrom
19prishpat:main
Closed

After Feedback version#20
19prishpat wants to merge 5 commits intoUWOrbital:mainfrom
19prishpat:main

Conversation

@19prishpat
Copy link

Purpose

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

New Changes

  • Explain new changes

Testing

  • Explain tests that you ran to verify code functionality.
  • Any functions that can be unit-tested should include a unit test in the PR. Otherwise, explain why it cannot be unit-tested.

Outstanding Changes

  • If there are non-critical changes (i.e. additional features) that can be made to this feature in the future, indicate them here.

Copy link
Contributor

@Yarik-Popov Yarik-Popov left a comment

Choose a reason for hiding this comment

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

Should be last set of changes

if self.params is None and self.format is None:
return self
if self.params is None or self.format is None:
print(f"[Validation Error] params: {self.params}, format: {self.format}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are raising an exception we shouldnt be printing. Same thing below

custom_logger.info(f"Query Params: {dict(request.query_params)}")


response = await call_next(request)
Copy link
Contributor

Choose a reason for hiding this comment

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

Print the response body as well for completeness

@Yarik-Popov
Copy link
Contributor

Congrats on finishing the GS backend on boarding. DM me your email and I'll add you to the notion (we only use it for the wiki and meeting minutes). I'll add u to the github org in a bit

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