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

refactor: Add log messages for BackEndApp/v1 #415

Closed
wants to merge 1 commit into from
Closed

refactor: Add log messages for BackEndApp/v1 #415

wants to merge 1 commit into from

Conversation

prajwalnayak7
Copy link

@prajwalnayak7 prajwalnayak7 commented Oct 5, 2021

Pull Request

What does this PR do?

Fixes #337

  • Added structured logging to all the views in BackEndApp/v1
  • Reused the messages wherever they existed
  • Removed print statements and replaced them with the logger

-->

What part does this affect?

  • FrontEnd.
  • BackEnd.
  • Documentation.
  • Other. (Please specify below)

Before submitting

  • Was this discussed/approved via a GitHub issue or slack?
  • Did you read the contributor guideline?
  • Did you ensure that there aren't any other open Pull Requests for the same update/change?
  • Did you make sure the title is self-explanatory and the description concisely explains the PR?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure the code is clean and docstrings have been added or updated as required?
  • Did you make sure the code is linted/formatted locally prior to submission? (using black and/or prettier)
  • Did you make sure to update the documentation with your changes? (if necessary)

PR review

Anyone in the community is free to review the PR once the tests have passed.

Thank you for contributing to AutoDL. We look forward to your continued support.

@prajwalnayak7 prajwalnayak7 changed the title logging: Add log messages for BackEndApp/v1 refactor: Add log messages for BackEndApp/v1 Oct 5, 2021
@McTechie McTechie requested a review from RusherRG October 5, 2021 04:54
BackEndApp/v1/views.py Outdated Show resolved Hide resolved
@@ -37,7 +39,9 @@ def generate(request):
project_id = request.data.get("project_id")
store_obj = Store(user)
if not store_obj.exist(project_id):
raise Exception("No such project exists")
error_message = "No such project exists"
logger.error(f"[API][POST][generate] {error_message}")
Copy link
Member

Choose a reason for hiding this comment

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

[API][POST][generate]

Is this supposed to resolve to some string?

Copy link
Author

Choose a reason for hiding this comment

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

Nope. It is a more structured way of logging. This will help in debugging (ex: filtering logs in the shell using grep).

Copy link
Member

@RusherRG RusherRG Oct 6, 2021

Choose a reason for hiding this comment

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

image
Could rather have a message like the above?

 2021-10-06 06:19:23 | v1.views/get_all_projects | INFO: "POST /v1/projects/all/ HTTP/1.1" Fetching all projects for 'test_user'

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@prajwalnayak7 could you make the necessary changes?

@McTechie
Copy link
Contributor

@prajwalnayak7 It would be great if you could alter the log messages to look like the one mentioned by @RusherRG in this discussion

@ADI10HERO
Copy link
Member

Hey @prajwalnayak7 closing the PR due to no activity. Please check the slack message in the #announcements channel for more info.

Thanks for your contributions! :D

@ADI10HERO ADI10HERO closed this Oct 20, 2021
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.

Add log messages for BackEndApp/v1
4 participants