feat: Adding loggers for workspaces api #681
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Ashutosh619-sudo
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
apps/workspaces/views.py (3)
63-65: LGTM! Consider adding structured logging.The added logging statements improve traceability and debugging capabilities. They provide relevant information about the user and workspace retrieval process.
Consider using structured logging for easier parsing and analysis:
logger.info('Workspace retrieval', extra={ 'user_id': request.user, 'workspace_details': workspaces[0].__dict__ if workspaces else None, 'org_id': org_id })This approach allows for more efficient log parsing and analysis in production environments.
Also applies to: 72-73
Line range hint
127-129: LGTM! Consider enhancing error handling.The added error handling improves the robustness of the OAuth connection process. The logging message provides useful information about the error, and the HTTP status code is appropriate.
Consider separating the error handling for different exception types to provide more specific error messages to the client:
except qbo_exc.UnauthorizedClientError as e: logger.error('Unauthorized client error', extra={'error': str(e)}) return Response({'message': 'Unauthorized client'}, status=status.HTTP_401_UNAUTHORIZED) except qbo_exc.NotFoundClientError as e: logger.error('QBO application not found', extra={'error': str(e)}) return Response({'message': 'QBO application not found'}, status=status.HTTP_404_NOT_FOUND) except (qbo_exc.WrongParamsError, qbo_exc.InternalServerError) as e: logger.error('QBO error', extra={'error': str(e)}) return Response({'message': 'An error occurred while connecting to QBO'}, status=status.HTTP_500_INTERNAL_SERVER_ERROR)This approach provides more specific error messages and appropriate status codes for different error scenarios.
Line range hint
137-140: LGTM! Consider enhancing the response structure.The added check for credential validity improves the reliability of the API. The use of the
is_expiredflag and the appropriate HTTP status code for invalid credentials are good practices.Consider enhancing the response structure to provide more informative feedback:
if qbo_credentials.refresh_token and not qbo_credentials.is_expired: return Response(data=QBOCredentialSerializer(qbo_credentials).data, status=status.HTTP_200_OK) else: return Response( data={ 'error': 'Invalid or expired QBO credentials', 'is_expired': qbo_credentials.is_expired, 'has_refresh_token': bool(qbo_credentials.refresh_token) }, status=status.HTTP_400_BAD_REQUEST )This approach provides more detailed information about why the credentials are considered invalid, which can be helpful for debugging and client-side error handling.
apps/workspaces/views.py
Outdated
| if workspaces: | ||
| logger.info('Workspace detail %s', workspaces[0].__dict__) |
There was a problem hiding this comment.
print whole workspaces variable, I am fine with it being ORM Returned object, id would still be visible
Ashutosh619-sudo
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
Ashutosh619-sudo
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
|
* feat: Adding loggers for workspaces api * comment resolved
* feat: Adding loggers for workspaces api * comment resolved
Description
Please add PR description here, add screenshots if needed
Clickup
Please add link here
https://app.clickup.com/1864988/v/l/6-901603904304-1
Summary by CodeRabbit
New Features
Bug Fixes