feat: Integration helper & Spotlight base branch #111
Conversation
Shwetabhk
had a problem deploying
to
CI Environment
GitHub Actions
Failure
WalkthroughThe changes introduce two new Django applications, Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Poem
Tip OpenAI O1 model for chat
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 (
|
Shwetabhk
had a problem deploying
to
CI Environment
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Outside diff range and nitpick comments (5)
apps/integration_helper/models.py (1)
4-15: LGTM! Consider removing the explicitidfield definition.The
Conversationmodel is well-structured and follows Django best practices. The field names are descriptive, and the field types are appropriately chosen. Thehelp_textparameter is effectively used to provide useful information about each field.However, since Django automatically adds an auto-incrementing primary key field named
idto each model, explicitly defining it is not necessary unless you want to customize the primary key behavior. Consider removing theidfield definition:class Conversation(models.Model): conversation_id = models.CharField(max_length=255, help_text="Unique id of the conversation") workspace_id = models.IntegerField(help_text="Workspace id of the organization") role = models.CharField(max_length=255, help_text="Role of the messenger") content = models.TextField(help_text="Content of the message") created_at = models.DateTimeField( auto_now_add=True, help_text="Created at datetime" ) class Meta: db_table = 'conversations'apps/spotlight/models.py (1)
9-23: TheQuerymodel implementation looks good with a few suggestions.
- Naming Convention:
- If the
_llm_responsefield is meant to be a public attribute, consider removing the leading underscore. In Python, a single leading underscore is used to indicate a "private" or "for internal use" attribute.
- JSONField Usage:
- Using a JSONField for the
_llm_responseprovides flexibility if the response structure is variable. However, if the response structure is well-defined and consistent, consider creating a separate model for the LLM response and establish a foreign key relationship with theQuerymodel. This promotes better data normalization and can make querying and filtering easier.Overall, the model follows good practices such as proper foreign key definition, usage of DateTimeFields, and explicit database table naming.
apps/spotlight/urls.py (1)
19-20: Remove extra blank lines.To improve readability, consider removing the extra blank lines.
Apply this diff to remove the extra blank lines:
- - urlpatterns = [apps/spotlight/migrations/0001_initial.py (1)
1-32: LGTM! Consider the naming convention for the_llm_responsefield.The migration file follows the standard structure and the
Querymodel fields are well-defined. The foreign key to theUsermodel is set up correctly with theon_deleteparameter set toCASCADE, ensuring data integrity.Regarding the
_llm_responsefield, the underscore prefix is a convention in Python for indicating private or internal attributes. However, it's important to note that this is just a naming convention and doesn't provide any actual access control or encapsulation.If the intention is to treat
_llm_responseas an internal field that should not be accessed directly from outside the model, consider using the@propertydecorator and a corresponding setter method to control access to this field. This way, you can add any necessary validation, transformation, or logging logic when the field is accessed or modified.For example:
class Query(models.Model): # ... _llm_response = models.JSONField(default={}) @property def llm_response(self): return self._llm_response @llm_response.setter def llm_response(self, value): # Add any necessary validation, transformation, or logging logic here self._llm_response = valueThis approach provides a cleaner and more explicit way to control access to the
_llm_responsefield.apps/spotlight/llm.py (1)
29-51: LGTM! Consider usingraise ... fromfor better exception handling.The function is well-structured and follows best practices for making API requests. It handles exceptions and re-raises them with a custom message, which is a good practice.
To further improve the exception handling, consider using
raise ... fromto distinguish errors in exception handling from the original exception.Apply this diff to use
raise ... from:- except (openai.OpenAIError, json.JSONDecodeError) as e: - raise Exception(message=str(e)) + except (openai.OpenAIError, json.JSONDecodeError) as e: + raise Exception(message=str(e)) from eTools
Ruff
51-51: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (28)
- apps/integration_helper/admin.py (1 hunks)
- apps/integration_helper/apps.py (1 hunks)
- apps/integration_helper/migrations/0001_initial.py (1 hunks)
- apps/integration_helper/migrations/0002_conversation_workspace_id.py (1 hunks)
- apps/integration_helper/models.py (1 hunks)
- apps/integration_helper/openai_utils.py (1 hunks)
- apps/integration_helper/prompt.py (1 hunks)
- apps/integration_helper/tests.py (1 hunks)
- apps/integration_helper/urls.py (1 hunks)
- apps/integration_helper/views.py (1 hunks)
- apps/spotlight/admin.py (1 hunks)
- apps/spotlight/apps.py (1 hunks)
- apps/spotlight/llm.py (1 hunks)
- apps/spotlight/migrations/0001_initial.py (1 hunks)
- apps/spotlight/models.py (1 hunks)
- apps/spotlight/prompts/spotlight_prompt.py (1 hunks)
- apps/spotlight/prompts/support_genie.py (1 hunks)
- apps/spotlight/serializers.py (1 hunks)
- apps/spotlight/service.py (1 hunks)
- apps/spotlight/tests.py (1 hunks)
- apps/spotlight/urls.py (1 hunks)
- apps/spotlight/views.py (1 hunks)
- apps/workspaces/tasks.py (1 hunks)
- apps/workspaces/urls.py (1 hunks)
- apps/workspaces/views.py (2 hunks)
- docker-compose.yml.template (1 hunks)
- quickbooks_desktop_api/settings.py (1 hunks)
- requirements.txt (2 hunks)
Files skipped from review due to trivial changes (1)
- apps/integration_helper/apps.py
Additional context used
Ruff
apps/integration_helper/tests.py
1-1:
django.test.TestCaseimported but unusedRemove unused import:
django.test.TestCase(F401)
apps/spotlight/tests.py
1-1:
django.test.TestCaseimported but unusedRemove unused import:
django.test.TestCase(F401)
apps/spotlight/admin.py
1-1:
django.contrib.adminimported but unusedRemove unused import:
django.contrib.admin(F401)
apps/integration_helper/openai_utils.py
3-3:
apps.integration_helper.prompt.PROMPTimported but unusedRemove unused import:
apps.integration_helper.prompt.PROMPT(F401)
apps/workspaces/urls.py
38-38: SyntaxError: Expected ',', found name
apps/spotlight/llm.py
51-51: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
apps/spotlight/service.py
87-87: Local variable
action_responseis assigned to but never usedRemove assignment to unused variable
action_response(F841)
apps/spotlight/views.py
5-5:
requestsimported but unusedRemove unused import:
requests(F401)
6-6:
django_q.tasks.async_taskimported but unusedRemove unused import:
django_q.tasks.async_task(F401)
12-12:
apps.workspaces.models.FyleCredentialimported but unusedRemove unused import:
apps.workspaces.models.FyleCredential(F401)
13-13:
apps.fyle.helpers.get_access_tokenimported but unusedRemove unused import:
apps.fyle.helpers.get_access_token(F401)
Additional comments not posted (30)
apps/spotlight/apps.py (1)
1-5: LGTM!The
SpotlightConfigclass is correctly defined by inheriting fromdjango.apps.AppConfigand setting thenameattribute to'spotlight'. No additional changes are needed.apps/integration_helper/admin.py (1)
1-6: LGTM!The file is correctly registering the
Conversationmodel in the Django admin. The necessary modules are imported, and the code follows the standard Django admin registration pattern.apps/integration_helper/urls.py (1)
1-7: LGTM!The URL configuration follows the standard Django pattern and is implemented correctly. The URL pattern maps the root URL to the
CoversationsViewand is named appropriately for reverse lookups.apps/spotlight/serializers.py (1)
6-9: LGTM!The
QuerySerializerclass is correctly defined and follows the Django REST framework conventions and best practices for defining serializers.apps/integration_helper/migrations/0002_conversation_workspace_id.py (1)
1-19: LGTM!The migration file follows the standard Django migration structure and naming convention. The
workspace_idfield is added to theConversationmodel with an appropriate field type and a helpful description. The migration file looks good and does not have any issues or potential problems.apps/spotlight/prompts/support_genie.py (1)
1-12: LGTM!The prompt is well-structured and provides clear instructions for the question-answering agent. It emphasizes the importance of providing factual responses based solely on the information in the search results, which is crucial for maintaining accuracy and avoiding assumptions.
The placeholders for the search results and output format instructions allow for easy integration with the rest of the system.
Overall, the prompt is a valuable addition to the codebase and does not introduce any issues.
apps/integration_helper/migrations/0001_initial.py (1)
1-27: LGTM!The migration file follows the standard structure and conventions. The
Conversationmodel fields are appropriately defined with relevant field types and help text. Thedb_tableoption is set correctly.requirements.txt (2)
1-3: LGTM!The new dependencies are relevant to the PR objectives and are pinned to specific versions, which is a good practice for reproducibility.
4-4: LGTM!The empty line improves readability by separating the new dependencies from the existing ones.
apps/spotlight/urls.py (1)
1-26: LGTM!The URL configuration follows the standard Django pattern and correctly maps the URLs to their respective views. The comments provide clear explanations and examples, making the file easy to understand.
docker-compose.yml.template (1)
28-32: LGTM! Ensure secure handling of sensitive values.The addition of the new environment variables for AWS, OpenAI, and knowledge base integration looks good. It's great that you're using placeholders in the template.
Please make sure to provide the actual values securely through a separate configuration mechanism (e.g., environment variables, secrets management) and avoid committing sensitive information to the repository.
apps/workspaces/urls.py (2)
37-37: LGTM!The new URL path for handling conversations associated with a specific workspace is correctly implemented. The path is properly namespaced under the workspace ID and includes the URL configurations from the new
integration_helperapplication, aligning with the PR objective.
38-38: LGTM!The new URL path for handling spotlight features associated with a specific workspace is correctly implemented. The path is properly namespaced under the workspace ID and includes the URL configurations from the new
spotlightapplication, aligning with the PR objective.Regarding the syntax error reported by the static analysis tool, it's a false positive. The code segment is the last item in the
urlpatternslist and does not require a trailing comma.Tools
Ruff
38-38: SyntaxError: Expected ',', found name
apps/spotlight/llm.py (1)
1-26: LGTM!The global variables and configurations are well-organized and follow best practices. Using environment variables for configuration is a good practice as it keeps sensitive information out of the codebase.
apps/workspaces/views.py (2)
19-19: LGTM!The import statement is consistent with the renamed function
trigger_exportinapps/workspaces/tasks.py.
101-101: Verify the behavior oftrigger_export.The code change is consistent with the renamed function and simplifies the logic by relying on the result of
trigger_export. This change could potentially streamline the process and improve maintainability by centralizing the export logic.To ensure the correctness of the changes, verify that
trigger_exporthandles the export and the associated checks as expected. Run the following script to retrieve the implementation oftrigger_export:Verification successful
Verified:
trigger_exporthandles export and checks correctly.The
trigger_exportfunction inapps/workspaces/tasks.pycorrectly handles the export logic by callingrun_import_exportand checks for new expenses that have not been exported. This confirms that the code change inapps/workspaces/views.pyis appropriate and maintains the intended functionality.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Retrieve the implementation of `trigger_export` to verify its behavior. # Test: Search for the function definition. ast-grep --lang python --pattern $'def trigger_export($$$)'Length of output: 405
Script:
#!/bin/bash # Description: Search for `trigger_export` across the codebase to find its definition or import statement. # Search for the definition or import of `trigger_export`. rg 'trigger_export'Length of output: 1054
Script:
#!/bin/bash # Description: Retrieve the implementation of `trigger_export` from `apps/workspaces/tasks.py`. # Extract the function definition and its body for `trigger_export`. ast-grep --lang python --pattern $'def trigger_export($$$)' --apps/workspaces/tasks.pyLength of output: 341
Script:
#!/bin/bash # Description: Retrieve the implementation of `trigger_export` from `apps/workspaces/tasks.py`. # Correct the command to extract the function definition and its body for `trigger_export`. ast-grep --lang python --pattern $'def trigger_export($$$)' apps/workspaces/tasks.pyLength of output: 430
Script:
#!/bin/bash # Description: Use `rg` to search for the `trigger_export` function definition in `apps/workspaces/tasks.py`. # Search for the function definition and its body for `trigger_export`. rg -A 10 'def trigger_export' apps/workspaces/tasks.pyLength of output: 289
apps/spotlight/service.py (2)
17-45: LGTM!The
HelpServiceclass is well-structured and follows the Single Responsibility Principle. Theextract_citationsandformat_responsemethods are correctly implemented and serve their intended purpose.
48-54: LGTM!The
QueryServiceclass is well-structured and follows the Single Responsibility Principle. Theget_suggestionsmethod is correctly implemented and serves its intended purpose.apps/spotlight/views.py (4)
38-60: LGTM!The
RecentQueryViewclass is correctly implemented and follows Django REST framework conventions. Thegetmethod correctly filters the queries based on the workspace ID and authenticated user, and returns the expected response containing only the query text.
63-78: LGTM!The
QueryViewclass is correctly implemented and follows Django REST framework conventions. Thepostmethod correctly creates a newQueryobject with the user query, workspace ID, suggestions, and user ID, and returns the expected response containing the suggestions for the user query.
81-86: LGTM!The
HelpQueryViewclass is correctly implemented and follows Django REST framework conventions. Thepostmethod correctly returns the expected response containing the support response for the user query.
89-100: LGTM!The
ActionQueryViewclass is correctly implemented and follows Django REST framework conventions. Thepostmethod correctly triggers the action based on the code provided by the user and returns the expected response. It also correctly handles exceptions and returns a failure message with a 500 status code.apps/workspaces/tasks.py (1)
105-110: LGTM!The
trigger_exportfunction is a great addition to the module. It follows the expected logic, is well-structured, and enhances the functionality by providing a mechanism to trigger exports and check for new data. The function name, parameters, and return value are all appropriately defined.apps/integration_helper/prompt.py (1)
1-126: LGTM!The prompt is well-structured, complete, clear, consistent, and maintainable. It provides a comprehensive guide for the expense management software assistant to gather user preferences and generate the final JSON payload. The step-by-step instructions, JSON formats, and guidelines are clearly specified, making it easy for the assistant to follow and implement the required functionality.
quickbooks_desktop_api/settings.py (1)
62-64: LGTM!The code changes correctly add the new
integration_helperandspotlightapps to theINSTALLED_APPSlist, which is necessary for integrating them into the project. This aligns with the stated PR objectives.apps/spotlight/prompts/spotlight_prompt.py (5)
6-45: LGTM!The general instructions section provides a clear and concise overview of the AI assistant's role and responsibilities. The expected format of the generated suggestions is well-defined, and the instructions cover important aspects such as handling ambiguity and returning empty arrays when necessary.
49-176: LGTM!The actions instructions provide clear guidelines for generating specific actions based on user queries. The mappings are comprehensive and cover a wide range of relevant actions related to export settings, field mappings, and expense management. The structure of the mappings is consistent, with each action having a unique code, title, and description.
179-221: LGTM!The navigations instructions provide clear guidelines for generating specific navigations based on user queries. The mappings are relevant and cover important navigations related to dashboards, settings, and field mappings. The structure of the mappings is consistent, with each navigation having a unique code, title, description, and URL.
224-276: LGTM!The help instructions provide clear guidelines for generating specific help suggestions based on user queries. The mappings cover a range of relevant help topics related to exports, field mappings, and settings. The structure of the mappings is consistent, with each help suggestion having a unique code, title, and description.
281-510: LGTM!The examples section is comprehensive and covers a wide range of scenarios related to exports, settings, field mappings, and automatic exports. The user input options are diverse and representative of potential user queries. The expected outputs are well-structured and follow the defined format, including relevant actions, navigations, and help suggestions.
| from django.test import TestCase | ||
|
|
||
| # Create your tests here. |
There was a problem hiding this comment.
Add test cases for the integration_helper app.
The test file is currently empty and does not contain any test cases. It's important to have a comprehensive test suite to ensure the correctness and reliability of the integration_helper app.
Do you want me to generate some test cases for the integration_helper app or open a GitHub issue to track this task?
Tools
Ruff
1-1:
django.test.TestCaseimported but unusedRemove unused import:
django.test.TestCase(F401)
| @@ -0,0 +1,3 @@ | |||
| from django.test import TestCase | |||
There was a problem hiding this comment.
Remove the unused import statement.
The TestCase import from django.test is unused in this file.
Apply this diff to remove the unused import:
-from django.test import TestCase
-Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from django.test import TestCase |
Tools
Ruff
1-1:
django.test.TestCaseimported but unusedRemove unused import:
django.test.TestCase(F401)
| from django.test import TestCase | ||
|
|
||
| # Create your tests here. |
There was a problem hiding this comment.
Remove unused import and add tests in the future.
It's great to see a new test file added! However, please remove the unused import of TestCase to keep the file clean.
Apply this diff to remove the unused import:
-from django.test import TestCase
-
# Create your tests here.Do you want me to help generate some initial test cases or open a GitHub issue to track adding tests for this app?
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from django.test import TestCase | |
| # Create your tests here. | |
| # Create your tests here. |
Tools
Ruff
1-1:
django.test.TestCaseimported but unusedRemove unused import:
django.test.TestCase(F401)
| @@ -0,0 +1,3 @@ | |||
| from django.contrib import admin | |||
There was a problem hiding this comment.
Remove the unused import.
The django.contrib.admin import is unused in this file. It's a good practice to remove unused imports to keep the code clean and maintainable.
Apply this diff to remove the unused import:
-from django.contrib import admin
-
# Register your models here.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from django.contrib import admin | |
| # Register your models here. |
Tools
Ruff
1-1:
django.contrib.adminimported but unusedRemove unused import:
django.contrib.admin(F401)
| def get_openai_response(messages): | ||
| """ | ||
| Send the conversation history (messages) to OpenAI and get a response. | ||
| """ | ||
| api_key = os.getenv("OPENAI_API_KEY") | ||
| client = OpenAI( | ||
| api_key=api_key | ||
| ) | ||
| response = client.chat.completions.create( | ||
| model="gpt-4o", | ||
| messages=messages, | ||
| response_format={"type": "json_object"}, | ||
| max_tokens=1000, | ||
| temperature=0, | ||
| ) | ||
|
|
||
| return json.loads(response.choices[0].message.content) |
There was a problem hiding this comment.
Add error handling and consider moving hardcoded parameters to configuration.
The function looks good overall, with a few suggestions for improvement:
-
Good job on using an environment variable for the API key. This keeps sensitive information out of the codebase. Just ensure that the
OPENAI_API_KEYenvironment variable is properly set in all environments where this code will run. -
Consider adding error handling for potential issues like missing API key, invalid API key, or network errors. This will prevent uncaught exceptions and make the function more resilient.
-
The function currently uses hardcoded parameters for the model, response format, max tokens, and temperature. While this may be intentional, consider moving these to configuration variables to make it easier to change them in the future if needed.
| def create(self, request, *args, **kwargs): | ||
| """ | ||
| Create a new conversation and get the first OpenAI response. | ||
| """ | ||
| content = request.data.get('content') | ||
| conversation_id = request.data.get('conversation_id') | ||
| workspace_id = kwargs['workspace_id'] | ||
|
|
||
| if not content: | ||
| return Response( | ||
| {'error': 'content are required'}, status=status.HTTP_400_BAD_REQUEST | ||
| ) | ||
|
|
||
| if not conversation_id: | ||
| conversation_id = str(uuid.uuid4()) | ||
|
|
||
| Conversation.objects.update_or_create( | ||
| defaults={'content': PROMPT}, | ||
| conversation_id=conversation_id, | ||
| workspace_id=workspace_id, | ||
| role='system' | ||
| ) | ||
|
|
||
| conversation = Conversation.objects.create( | ||
| conversation_id=conversation_id, | ||
| workspace_id=workspace_id, | ||
| role='user', | ||
| content=content | ||
| ) | ||
|
|
||
| messages = list( | ||
| Conversation.objects.filter( | ||
| conversation_id=conversation_id, | ||
| workspace_id=workspace_id | ||
| ).values('role', 'content').order_by('created_at')) | ||
|
|
||
| assistant_response = get_openai_response(messages) | ||
|
|
||
| Conversation.objects.create( | ||
| conversation_id=conversation_id, | ||
| workspace_id=workspace_id, | ||
| role='assistant', | ||
| content=assistant_response, | ||
| ) | ||
|
|
||
| return Response( | ||
| { | ||
| 'conversation_id': conversation.conversation_id, | ||
| 'content': assistant_response, | ||
| }, | ||
| status=status.HTTP_201_CREATED, | ||
| ) |
There was a problem hiding this comment.
LGTM! Consider extracting the OpenAI API interaction logic and improving error handling.
The create method follows a clear sequence of steps to create a new conversation and retrieve the assistant response from the OpenAI API. The implementation looks good overall.
Suggestions for further improvement:
- Consider extracting the OpenAI API interaction logic (lines 44-50) into a separate method for better code organization and reusability.
- Add more detailed error handling and logging throughout the method to facilitate better troubleshooting and monitoring.
| def delete(self, request, *args, **kwargs): | ||
| """ | ||
| Clear the conversation history by deleting it using conversation_id. | ||
| """ | ||
| workspace_id = kwargs['workspace_id'] | ||
| conversation_id = request.data.get('conversation_id') | ||
| if not conversation_id: | ||
| return Response( | ||
| { | ||
| 'error': 'conversation_id is required' | ||
| }, status=status.HTTP_400_BAD_REQUEST | ||
| ) | ||
|
|
||
| conversations = Conversation.objects.filter( | ||
| conversation_id=conversation_id, | ||
| workspace_id=workspace_id | ||
| ) | ||
|
|
||
| if conversations.exists(): | ||
| conversations.delete() | ||
|
|
||
| return Response( | ||
| { | ||
| 'message': 'Conversation cleared' | ||
| }, status=status.HTTP_200_OK | ||
| ) |
There was a problem hiding this comment.
LGTM! Consider adding a confirmation step and a more informative response.
The delete method correctly handles the deletion of conversations based on the provided conversation_id and workspace_id. The implementation looks good overall.
Suggestions for further improvement:
- Consider adding a confirmation step before deleting the conversations to prevent accidental deletions. This could be achieved by requiring an additional parameter in the request data to confirm the deletion.
- Instead of returning a generic success message, consider returning a more informative response indicating the number of conversations deleted. This would provide better feedback to the client about the outcome of the operation.
| def get_support_response_from_bedrock(*, prompt_template: str, input_message: str) -> Dict: | ||
| try: | ||
| bedrock_agent_runtime_client = bedrock_session.client( | ||
| 'bedrock-agent-runtime' | ||
| ) | ||
|
|
||
| response = bedrock_agent_runtime_client.retrieve_and_generate( | ||
| input={ | ||
| 'text': input_message | ||
| }, | ||
| retrieveAndGenerateConfiguration={ | ||
| 'type': 'KNOWLEDGE_BASE', | ||
| 'knowledgeBaseConfiguration': { | ||
| 'knowledgeBaseId': KNOWLEDGE_BASE_ID, | ||
| 'modelArn': 'arn:aws:bedrock:ap-south-1::foundation-model/anthropic.claude-3-sonnet-20240229-v1:0', | ||
| 'generationConfiguration': { | ||
| 'inferenceConfig': { | ||
| 'textInferenceConfig': { | ||
| 'maxTokens': 2048, | ||
| 'stopSequences': [], | ||
| 'temperature': 0, | ||
| 'topP': 1 | ||
| } | ||
| }, | ||
| 'promptTemplate': { | ||
| 'textPromptTemplate': prompt_template | ||
| } | ||
| }, | ||
| 'retrievalConfiguration': { | ||
| 'vectorSearchConfiguration': { | ||
| 'numberOfResults': 5, | ||
| 'overrideSearchType': 'HYBRID', | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ) | ||
|
|
||
| return response | ||
|
|
||
| except json.JSONDecodeError as e: | ||
| print(e) |
There was a problem hiding this comment.
LGTM! Consider refactoring the function to improve readability.
The function is well-structured and follows best practices for making API requests. It handles JSON decoding errors and prints them, which is a good practice.
However, the function is quite long and could benefit from some refactoring to improve readability.
Consider extracting the retrieveAndGenerateConfiguration dictionary into a separate function or variable to improve readability. For example:
def get_retrieve_and_generate_config(prompt_template):
return {
'type': 'KNOWLEDGE_BASE',
'knowledgeBaseConfiguration': {
'knowledgeBaseId': KNOWLEDGE_BASE_ID,
'modelArn': 'arn:aws:bedrock:ap-south-1::foundation-model/anthropic.claude-3-sonnet-20240229-v1:0',
'generationConfiguration': {
'inferenceConfig': {
'textInferenceConfig': {
'maxTokens': 2048,
'stopSequences': [],
'temperature': 0,
'topP': 1
}
},
'promptTemplate': {
'textPromptTemplate': prompt_template
}
},
'retrievalConfiguration': {
'vectorSearchConfiguration': {
'numberOfResults': 5,
'overrideSearchType': 'HYBRID',
}
}
}
}
def get_support_response_from_bedrock(*, prompt_template: str, input_message: str) -> Dict:
try:
bedrock_agent_runtime_client = bedrock_session.client(
'bedrock-agent-runtime'
)
response = bedrock_agent_runtime_client.retrieve_and_generate(
input={
'text': input_message
},
retrieveAndGenerateConfiguration=get_retrieve_and_generate_config(prompt_template)
)
return response
except json.JSONDecodeError as e:
print(e)
apps/spotlight/service.py
Outdated
| class ActionService: | ||
|
|
||
| @classmethod | ||
| def _get_action_function_from_code(cls, *, code: str) -> Callable: | ||
| code_to_function_map = { | ||
| "trigger_export": cls.trigger_export | ||
| } | ||
| return code_to_function_map[code] | ||
|
|
||
| @classmethod | ||
| def get_headers(cls, *, access_token: str) -> Dict: | ||
| return { | ||
| "Authorization": f"Bearer {access_token}", | ||
| "Content-Type": "application/json" | ||
| } | ||
|
|
||
| @classmethod | ||
| def get_access_token(cls, *, workspace_id: int) -> str: | ||
| creds = FyleCredential.objects.get(workspace_id=workspace_id) | ||
| return get_access_token(creds.refresh_token) | ||
|
|
||
| @classmethod | ||
| def trigger_export(cls, *, workspace_id: int): | ||
| access_token = cls.get_access_token(workspace_id=workspace_id) | ||
| headers = cls.get_headers(access_token=access_token) | ||
| headers = { | ||
| "Authorization": f"Bearer {access_token}", | ||
| "Content-Type": "application/json" | ||
| } | ||
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/trigger_export/' | ||
| action_response = requests.post(url, json={}, headers=headers) | ||
|
|
||
| @classmethod | ||
| def action(cls, *, code: str, workspace_id: str): | ||
| action_function = cls._get_action_function_from_code(code=code) | ||
| action_function(workspace_id=workspace_id) |
There was a problem hiding this comment.
Remove the unused variable action_response in the trigger_export method.
The ActionService class is well-structured and follows the Single Responsibility Principle. The class methods are correctly implemented and serve their intended purpose.
However, the static analysis tool has correctly flagged an unused variable action_response in the trigger_export method.
Apply this diff to remove the unused variable:
- action_response = requests.post(url, json={}, headers=headers)
+ requests.post(url, json={}, headers=headers)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class ActionService: | |
| @classmethod | |
| def _get_action_function_from_code(cls, *, code: str) -> Callable: | |
| code_to_function_map = { | |
| "trigger_export": cls.trigger_export | |
| } | |
| return code_to_function_map[code] | |
| @classmethod | |
| def get_headers(cls, *, access_token: str) -> Dict: | |
| return { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| @classmethod | |
| def get_access_token(cls, *, workspace_id: int) -> str: | |
| creds = FyleCredential.objects.get(workspace_id=workspace_id) | |
| return get_access_token(creds.refresh_token) | |
| @classmethod | |
| def trigger_export(cls, *, workspace_id: int): | |
| access_token = cls.get_access_token(workspace_id=workspace_id) | |
| headers = cls.get_headers(access_token=access_token) | |
| headers = { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/trigger_export/' | |
| action_response = requests.post(url, json={}, headers=headers) | |
| @classmethod | |
| def action(cls, *, code: str, workspace_id: str): | |
| action_function = cls._get_action_function_from_code(code=code) | |
| action_function(workspace_id=workspace_id) | |
| class ActionService: | |
| @classmethod | |
| def _get_action_function_from_code(cls, *, code: str) -> Callable: | |
| code_to_function_map = { | |
| "trigger_export": cls.trigger_export | |
| } | |
| return code_to_function_map[code] | |
| @classmethod | |
| def get_headers(cls, *, access_token: str) -> Dict: | |
| return { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| @classmethod | |
| def get_access_token(cls, *, workspace_id: int) -> str: | |
| creds = FyleCredential.objects.get(workspace_id=workspace_id) | |
| return get_access_token(creds.refresh_token) | |
| @classmethod | |
| def trigger_export(cls, *, workspace_id: int): | |
| access_token = cls.get_access_token(workspace_id=workspace_id) | |
| headers = cls.get_headers(access_token=access_token) | |
| headers = { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/trigger_export/' | |
| requests.post(url, json={}, headers=headers) | |
| @classmethod | |
| def action(cls, *, code: str, workspace_id: str): | |
| action_function = cls._get_action_function_from_code(code=code) | |
| action_function(workspace_id=workspace_id) |
Tools
Ruff
87-87: Local variable
action_responseis assigned to but never usedRemove assignment to unused variable
action_response(F841)
apps/spotlight/views.py
Outdated
| import json | ||
| from django.http import JsonResponse | ||
| from django.db import transaction | ||
| from rest_framework import generics | ||
| import requests | ||
| from django_q.tasks import async_task | ||
|
|
||
| from apps.spotlight.models import Query | ||
| from apps.spotlight.serializers import QuerySerializer | ||
|
|
||
| from .service import ActionService, HelpService, QueryService | ||
| from apps.workspaces.models import FyleCredential | ||
| from apps.fyle.helpers import get_access_token |
There was a problem hiding this comment.
Remove unused imports.
The static analysis tool Ruff has correctly flagged the following unused imports:
requests(line 5)django_q.tasks.async_task(line 6)apps.workspaces.models.FyleCredential(line 12)apps.fyle.helpers.get_access_token(line 13)
These imports should be removed to keep the codebase clean and maintainable.
Apply this diff to remove the unused imports:
-import requests
-from django_q.tasks import async_task
-from apps.workspaces.models import FyleCredential
-from apps.fyle.helpers import get_access_tokenCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import json | |
| from django.http import JsonResponse | |
| from django.db import transaction | |
| from rest_framework import generics | |
| import requests | |
| from django_q.tasks import async_task | |
| from apps.spotlight.models import Query | |
| from apps.spotlight.serializers import QuerySerializer | |
| from .service import ActionService, HelpService, QueryService | |
| from apps.workspaces.models import FyleCredential | |
| from apps.fyle.helpers import get_access_token | |
| import json | |
| from django.http import JsonResponse | |
| from django.db import transaction | |
| from rest_framework import generics | |
| from apps.spotlight.models import Query | |
| from apps.spotlight.serializers import QuerySerializer | |
| from .service import ActionService, HelpService, QueryService |
Tools
Ruff
5-5:
requestsimported but unusedRemove unused import:
requests(F401)
6-6:
django_q.tasks.async_taskimported but unusedRemove unused import:
django_q.tasks.async_task(F401)
12-12:
apps.workspaces.models.FyleCredentialimported but unusedRemove unused import:
apps.workspaces.models.FyleCredential(F401)
13-13:
apps.fyle.helpers.get_access_tokenimported but unusedRemove unused import:
apps.fyle.helpers.get_access_token(F401)
Shwetabhk
had a problem deploying
to
CI Environment
GitHub Actions
Failure
|
anishfyle
added
deploy
anishfyle
had a problem deploying
to
CI Environment
GitHub Actions
Failure
Ashutosh619-sudo
had a problem deploying
to
CI Environment
GitHub Actions
Failure
Ashutosh619-sudo
added
deploy
Ashutosh619-sudo
had a problem deploying
to
CI Environment
GitHub Actions
Failure
Ashutosh619-sudo
temporarily deployed
to
Staging
GitHub Actions
Inactive
|
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range and nitpick comments (1)
apps/spotlight/views.py (1)
40-62: LGTM!The
RecentQueryViewclass is correctly implemented and follows Django REST framework conventions. Thegetmethod correctly filters the queries based on the provided workspace ID and user, and returns the results as a JSON response.Consider removing the commented-out code at lines 55-60 as it is not being used:
- # recent_queries = [] - # for query in _recent_queries: - # recent_queries.append({ - # "query": query.query, - # "suggestions": query._llm_response["suggestions"] - # })
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- apps/spotlight/migrations/0002_copyexportsettings.py (1 hunks)
- apps/spotlight/models.py (1 hunks)
- apps/spotlight/prompts/suggestion_context_page_prompt.py (1 hunks)
- apps/spotlight/service.py (1 hunks)
- apps/spotlight/urls.py (1 hunks)
- apps/spotlight/views.py (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- apps/spotlight/models.py
- apps/spotlight/urls.py
Additional context used
Ruff
apps/spotlight/views.py
5-5:
requestsimported but unusedRemove unused import:
requests(F401)
6-6:
django_q.tasks.async_taskimported but unusedRemove unused import:
django_q.tasks.async_task(F401)
12-12:
apps.workspaces.models.FyleCredentialimported but unusedRemove unused import:
apps.workspaces.models.FyleCredential(F401)
13-13:
apps.fyle.helpers.get_access_tokenimported but unusedRemove unused import:
apps.fyle.helpers.get_access_token(F401)
14-14:
rest_framework.response.Responseimported but unusedRemove unused import:
rest_framework.response.Response(F401)
apps/spotlight/service.py
274-274: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
292-292: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
310-310: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
328-328: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
Additional comments not posted (18)
apps/spotlight/migrations/0002_copyexportsettings.py (4)
16-16: LGTM!The
idfield is correctly defined as an auto-incrementing primary key using theAutoFieldwith appropriate options.
17-17: LGTM!The
workspace_idfield is correctly defined as an integer field with a helpful text description.
18-18: LGTM!The
reimbursable_export_settingfield is correctly defined as a JSON field that can be null using theJSONFieldwith thenull=Trueoption.
19-19: LGTM!The
ccc_export_settingfield is correctly defined as a JSON field that can be null using theJSONFieldwith thenull=Trueoption.apps/spotlight/views.py (4)
65-80: LGTM!The
QueryViewclass is correctly implemented and follows Django REST framework conventions. Thepostmethod correctly creates a new query based on the provided workspace ID, user, and query text, and returns the suggestions as a JSON response. The use of an atomic transaction ensures data consistency.
83-88: LGTM!The
HelpQueryViewclass is correctly implemented and follows Django REST framework conventions. Thepostmethod correctly retrieves the support response based on the provided user query and returns it as a JSON response.
91-105: LGTM!The
ActionQueryViewclass is correctly implemented and follows Django REST framework conventions. Thepostmethod correctly performs the action based on the provided code and workspace ID, and returns a success or failure message as a JSON response. The method also handles exceptions and returns a failure message with a 500 status code in case of an error.
107-113: LGTM!The
SuggestionForPageclass is correctly implemented and follows Django REST framework conventions. Thepostmethod correctly retrieves the suggestions based on the provided user query and returns them as a JSON response.apps/spotlight/prompts/suggestion_context_page_prompt.py (5)
3-11: LGTM!The objectives and instructions are clear, concise, and provide a good overview of the AI agent's purpose and expected input/output format.
13-25: Example output for/dashboardlooks good!The example follows the specified JSON format and includes a relevant action for exporting data, which is appropriate for a dashboard page.
28-126: Comprehensive example output for/export_settings!The example covers a wide range of relevant actions for managing export settings, with clear titles and descriptions for each action. The consistent structure and naming convention for the actions is also appreciated.
128-155: Example output for/field_mappingsis on point!The example includes relevant actions for mapping fields, with clear descriptions and the expected JSON structure.
158-175: Important considerations and user query handling look solid!The guidelines provided in the important considerations section are clear and cover key scenarios the AI agent should handle. The use of a placeholder for the user query is also a good practice.
apps/spotlight/service.py (5)
24-43: LGTM!The
HelpServiceclass is well-structured and follows the Single Responsibility Principle. The class methods are correctly implemented and serve their intended purpose.
55-61: LGTM!The
QueryServiceclass is well-structured and follows the Single Responsibility Principle. The class method is correctly implemented and serves its intended purpose.
63-70: LGTM!The
SuggestionServiceclass is well-structured and follows the Single Responsibility Principle. The class method is correctly implemented and serves its intended purpose.
72-433: LGTM!The
ActionServiceclass is well-structured and follows the Single Responsibility Principle. The class methods are correctly implemented and serve their intended purpose.Tools
Ruff
274-274: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
292-292: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
310-310: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
328-328: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
190-201: Remove the unused variableaction_responsein thetrigger_exportmethod.The static analysis tool has correctly flagged an unused variable
action_responsein thetrigger_exportmethod.Apply this diff to remove the unused variable:
- action_response = requests.post(url, json={}, headers=headers) + requests.post(url, json={}, headers=headers)
| from django.http import JsonResponse | ||
| from django.db import transaction | ||
| from rest_framework import generics | ||
| import requests |
There was a problem hiding this comment.
Remove unused imports.
The static analysis tool Ruff has correctly flagged the following unused imports:
requests(line 5)django_q.tasks.async_task(line 6)apps.workspaces.models.FyleCredential(line 12)apps.fyle.helpers.get_access_token(line 13)rest_framework.response.Response(line 14)
These imports should be removed to keep the codebase clean and maintainable.
Apply this diff to remove the unused imports:
-import requests
-from django_q.tasks import async_task
-from apps.workspaces.models import FyleCredential
-from apps.fyle.helpers import get_access_token
-from rest_framework.response import ResponseAlso applies to: 6-6, 12-12, 13-13, 14-14
Tools
Ruff
5-5:
requestsimported but unusedRemove unused import:
requests(F401)
apps/spotlight/service.py
Outdated
| @classmethod | ||
| def set_customer_field_mapping_to_cost_center(cls, *, workspace_id: int): | ||
| access_token = cls.get_access_token(workspace_id=workspace_id) | ||
| headers = cls.get_headers(access_token=access_token) | ||
| headers = { | ||
| "Authorization": f"Bearer {access_token}", | ||
| "Content-Type": "application/json" | ||
| } | ||
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/field_mappings/' | ||
|
|
||
| action_response = requests.get(url, headers=headers) | ||
| action_response= action_response.json() | ||
| if action_response.get('project_type') != 'COST_CENTER' and action_response.get('class_type') != 'PROJECT': | ||
| action_response['project_type'] = 'COST_CENTER' | ||
| post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) | ||
| return ActionResponse(message="Field mapping updated successfully", is_success=True) | ||
| return ActionResponse(message="Field mapping already exists", is_success=False) |
There was a problem hiding this comment.
Remove the unused variable post_response.
The set_customer_field_mapping_to_cost_center method is correctly implemented and serves its intended purpose.
However, as mentioned in the static analysis hint, the variable post_response is assigned but never used.
Apply this diff to remove the unused variable:
- post_response = requests.post(url, headers=headers, data=json.dumps(action_response))
+ requests.post(url, headers=headers, data=json.dumps(action_response))Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @classmethod | |
| def set_customer_field_mapping_to_cost_center(cls, *, workspace_id: int): | |
| access_token = cls.get_access_token(workspace_id=workspace_id) | |
| headers = cls.get_headers(access_token=access_token) | |
| headers = { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/field_mappings/' | |
| action_response = requests.get(url, headers=headers) | |
| action_response= action_response.json() | |
| if action_response.get('project_type') != 'COST_CENTER' and action_response.get('class_type') != 'PROJECT': | |
| action_response['project_type'] = 'COST_CENTER' | |
| post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) | |
| return ActionResponse(message="Field mapping updated successfully", is_success=True) | |
| return ActionResponse(message="Field mapping already exists", is_success=False) | |
| @classmethod | |
| def set_customer_field_mapping_to_cost_center(cls, *, workspace_id: int): | |
| access_token = cls.get_access_token(workspace_id=workspace_id) | |
| headers = cls.get_headers(access_token=access_token) | |
| headers = { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/field_mappings/' | |
| action_response = requests.get(url, headers=headers) | |
| action_response= action_response.json() | |
| if action_response.get('project_type') != 'COST_CENTER' and action_response.get('class_type') != 'PROJECT': | |
| action_response['project_type'] = 'COST_CENTER' | |
| requests.post(url, headers=headers, data=json.dumps(action_response)) | |
| return ActionResponse(message="Field mapping updated successfully", is_success=True) | |
| return ActionResponse(message="Field mapping already exists", is_success=False) |
Tools
Ruff
292-292: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
apps/spotlight/service.py
Outdated
|
|
||
| @classmethod | ||
| def set_customer_field_mapping_to_project(cls, *, workspace_id: int): | ||
| access_token = cls.get_access_token(workspace_id=workspace_id) | ||
| headers = cls.get_headers(access_token=access_token) | ||
| headers = { | ||
| "Authorization": f"Bearer {access_token}", | ||
| "Content-Type": "application/json" | ||
| } | ||
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/field_mappings/' | ||
| headers = { | ||
| "Authorization": f"Bearer {access_token}", | ||
| "Content-Type": "application/json" | ||
| } | ||
| action_response = requests.get(url, headers=headers) | ||
| action_response= action_response.json() | ||
| if action_response.get('project_type') != 'PROJECT' and action_response.get('class_type') != 'COST_CENTER': | ||
| action_response['project_type'] = 'PROJECT' | ||
| post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) | ||
| return ActionResponse(message="Field mapping updated successfully", is_success=True) | ||
| return ActionResponse(message="Field mapping already exists", is_success=False) |
There was a problem hiding this comment.
Remove the unused variable post_response.
The set_customer_field_mapping_to_project method is correctly implemented and serves its intended purpose.
However, as mentioned in the static analysis hint, the variable post_response is assigned but never used.
Apply this diff to remove the unused variable:
- post_response = requests.post(url, headers=headers, data=json.dumps(action_response))
+ requests.post(url, headers=headers, data=json.dumps(action_response))Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @classmethod | |
| def set_customer_field_mapping_to_project(cls, *, workspace_id: int): | |
| access_token = cls.get_access_token(workspace_id=workspace_id) | |
| headers = cls.get_headers(access_token=access_token) | |
| headers = { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/field_mappings/' | |
| headers = { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| action_response = requests.get(url, headers=headers) | |
| action_response= action_response.json() | |
| if action_response.get('project_type') != 'PROJECT' and action_response.get('class_type') != 'COST_CENTER': | |
| action_response['project_type'] = 'PROJECT' | |
| post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) | |
| return ActionResponse(message="Field mapping updated successfully", is_success=True) | |
| return ActionResponse(message="Field mapping already exists", is_success=False) | |
| @classmethod | |
| def set_customer_field_mapping_to_project(cls, *, workspace_id: int): | |
| access_token = cls.get_access_token(workspace_id=workspace_id) | |
| headers = cls.get_headers(access_token=access_token) | |
| headers = { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/field_mappings/' | |
| headers = { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| action_response = requests.get(url, headers=headers) | |
| action_response= action_response.json() | |
| if action_response.get('project_type') != 'PROJECT' and action_response.get('class_type') != 'COST_CENTER': | |
| action_response['project_type'] = 'PROJECT' | |
| requests.post(url, headers=headers, data=json.dumps(action_response)) | |
| return ActionResponse(message="Field mapping updated successfully", is_success=True) | |
| return ActionResponse(message="Field mapping already exists", is_success=False) |
Tools
Ruff
274-274: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
apps/spotlight/service.py
Outdated
| @classmethod | ||
| def set_class_field_mapping_to_cost_center(cls, *, workspace_id: int): | ||
| access_token = cls.get_access_token(workspace_id=workspace_id) | ||
| headers = cls.get_headers(access_token=access_token) | ||
| headers = { | ||
| "Authorization": f"Bearer {access_token}", | ||
| "Content-Type": "application/json" | ||
| } | ||
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/field_mappings/' | ||
|
|
||
| action_response = requests.get(url, headers=headers) | ||
| action_response= action_response.json() | ||
| if action_response.get('project_type') != 'COST_CENTER' and action_response.get('class_type') != 'PROJECT': | ||
| action_response['class_type'] = 'COST_CENTER' | ||
| post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) | ||
| return ActionResponse(message="Field mapping updated successfully", is_success=True) | ||
| return ActionResponse(message="Field mapping already exists", is_success=False) |
There was a problem hiding this comment.
Remove the unused variable post_response.
The set_class_field_mapping_to_cost_center method is correctly implemented and serves its intended purpose.
However, as mentioned in the static analysis hint, the variable post_response is assigned but never used.
Apply this diff to remove the unused variable:
- post_response = requests.post(url, headers=headers, data=json.dumps(action_response))
+ requests.post(url, headers=headers, data=json.dumps(action_response))Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @classmethod | |
| def set_class_field_mapping_to_cost_center(cls, *, workspace_id: int): | |
| access_token = cls.get_access_token(workspace_id=workspace_id) | |
| headers = cls.get_headers(access_token=access_token) | |
| headers = { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/field_mappings/' | |
| action_response = requests.get(url, headers=headers) | |
| action_response= action_response.json() | |
| if action_response.get('project_type') != 'COST_CENTER' and action_response.get('class_type') != 'PROJECT': | |
| action_response['class_type'] = 'COST_CENTER' | |
| post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) | |
| return ActionResponse(message="Field mapping updated successfully", is_success=True) | |
| return ActionResponse(message="Field mapping already exists", is_success=False) | |
| @classmethod | |
| def set_class_field_mapping_to_cost_center(cls, *, workspace_id: int): | |
| access_token = cls.get_access_token(workspace_id=workspace_id) | |
| headers = cls.get_headers(access_token=access_token) | |
| headers = { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/field_mappings/' | |
| action_response = requests.get(url, headers=headers) | |
| action_response= action_response.json() | |
| if action_response.get('project_type') != 'COST_CENTER' and action_response.get('class_type') != 'PROJECT': | |
| action_response['class_type'] = 'COST_CENTER' | |
| requests.post(url, headers=headers, data=json.dumps(action_response)) | |
| return ActionResponse(message="Field mapping updated successfully", is_success=True) | |
| return ActionResponse(message="Field mapping already exists", is_success=False) |
Tools
Ruff
328-328: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
apps/spotlight/service.py
Outdated
| @classmethod | ||
| def set_class_field_mapping_to_project(cls, *, workspace_id: int): | ||
| access_token = cls.get_access_token(workspace_id=workspace_id) | ||
| headers = cls.get_headers(access_token=access_token) | ||
| headers = { | ||
| "Authorization": f"Bearer {access_token}", | ||
| "Content-Type": "application/json" | ||
| } | ||
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/field_mappings/' | ||
|
|
||
| action_response = requests.get(url, headers=headers) | ||
| action_response= action_response.json() | ||
| if action_response.get('project_type') != 'PROJECT' and action_response.get('class_type') != 'COST_CENTER': | ||
| action_response['class_type'] = 'PROJECT' | ||
| post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) | ||
| return ActionResponse(message="Field mapping updated successfully", is_success=True) | ||
| return ActionResponse(message="Field mapping already exists", is_success=False) |
There was a problem hiding this comment.
Remove the unused variable post_response.
The set_class_field_mapping_to_project method is correctly implemented and serves its intended purpose.
However, as mentioned in the static analysis hint, the variable post_response is assigned but never used.
Apply this diff to remove the unused variable:
- post_response = requests.post(url, headers=headers, data=json.dumps(action_response))
+ requests.post(url, headers=headers, data=json.dumps(action_response))Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @classmethod | |
| def set_class_field_mapping_to_project(cls, *, workspace_id: int): | |
| access_token = cls.get_access_token(workspace_id=workspace_id) | |
| headers = cls.get_headers(access_token=access_token) | |
| headers = { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/field_mappings/' | |
| action_response = requests.get(url, headers=headers) | |
| action_response= action_response.json() | |
| if action_response.get('project_type') != 'PROJECT' and action_response.get('class_type') != 'COST_CENTER': | |
| action_response['class_type'] = 'PROJECT' | |
| post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) | |
| return ActionResponse(message="Field mapping updated successfully", is_success=True) | |
| return ActionResponse(message="Field mapping already exists", is_success=False) | |
| @classmethod | |
| def set_class_field_mapping_to_project(cls, *, workspace_id: int): | |
| access_token = cls.get_access_token(workspace_id=workspace_id) | |
| headers = cls.get_headers(access_token=access_token) | |
| headers = { | |
| "Authorization": f"Bearer {access_token}", | |
| "Content-Type": "application/json" | |
| } | |
| url = f'http://localhost:8000/api/workspaces/{workspace_id}/field_mappings/' | |
| action_response = requests.get(url, headers=headers) | |
| action_response= action_response.json() | |
| if action_response.get('project_type') != 'PROJECT' and action_response.get('class_type') != 'COST_CENTER': | |
| action_response['class_type'] = 'PROJECT' | |
| requests.post(url, headers=headers, data=json.dumps(action_response)) | |
| return ActionResponse(message="Field mapping updated successfully", is_success=True) | |
| return ActionResponse(message="Field mapping already exists", is_success=False) |
Tools
Ruff
310-310: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
Ashutosh619-sudo
had a problem deploying
to
CI Environment
GitHub Actions
Failure
Ashutosh619-sudo
added
deploy
Ashutosh619-sudo
had a problem deploying
to
CI Environment
GitHub Actions
Failure
Ashutosh619-sudo
temporarily deployed
to
Staging
GitHub Actions
Inactive
|
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/spotlight/service.py (1 hunks)
Additional context used
Ruff
apps/spotlight/service.py
274-274: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
292-292: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
310-310: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
328-328: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
Additional comments not posted (8)
apps/spotlight/service.py (8)
24-53: LGTM!The
HelpServiceclass is well-structured and follows the Single Responsibility Principle. The class methods are correctly implemented and serve their intended purpose.
55-62: LGTM!The
QueryServiceclass is well-structured and follows the Single Responsibility Principle. The class method is correctly implemented and serves its intended purpose.
63-71: LGTM!The
SuggestionServiceclass is well-structured and follows the Single Responsibility Principle. The class method is correctly implemented and serves its intended purpose.
72-433: LGTM!The
ActionServiceclass is well-structured and follows the Single Responsibility Principle. The class methods are correctly implemented and serve their intended purpose.Tools
Ruff
274-274: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
292-292: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
310-310: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
328-328: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
18-22: LGTM!The
ActionResponsedataclass is well-structured and follows the Single Responsibility Principle. The dataclass is correctly implemented and serves its intended purpose.
292-292: Remove the unused variablepost_responsein theset_customer_field_mapping_to_cost_centermethod.The static analysis tool has correctly flagged an unused variable
post_responsein theset_customer_field_mapping_to_cost_centermethod.Apply this diff to remove the unused variable:
- post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) + requests.post(url, headers=headers, data=json.dumps(action_response))Tools
Ruff
292-292: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
310-310: Remove the unused variablepost_responsein theset_class_field_mapping_to_projectmethod.The static analysis tool has correctly flagged an unused variable
post_responsein theset_class_field_mapping_to_projectmethod.Apply this diff to remove the unused variable:
- post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) + requests.post(url, headers=headers, data=json.dumps(action_response))Tools
Ruff
310-310: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
328-328: Remove the unused variablepost_responsein theset_class_field_mapping_to_cost_centermethod.The static analysis tool has correctly flagged an unused variable
post_responsein theset_class_field_mapping_to_cost_centermethod.Apply this diff to remove the unused variable:
- post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) + requests.post(url, headers=headers, data=json.dumps(action_response))Tools
Ruff
328-328: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
| action_response= action_response.json() | ||
| if action_response.get('project_type') != 'PROJECT' and action_response.get('class_type') != 'COST_CENTER': | ||
| action_response['project_type'] = 'PROJECT' | ||
| post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) |
There was a problem hiding this comment.
Remove the unused variable post_response in the set_customer_field_mapping_to_project method.
The static analysis tool has correctly flagged an unused variable post_response in the set_customer_field_mapping_to_project method.
Apply this diff to remove the unused variable:
- post_response = requests.post(url, headers=headers, data=json.dumps(action_response))
+ requests.post(url, headers=headers, data=json.dumps(action_response))Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| post_response = requests.post(url, headers=headers, data=json.dumps(action_response)) | |
| requests.post(url, headers=headers, data=json.dumps(action_response)) |
Tools
Ruff
274-274: Local variable
post_responseis assigned to but never usedRemove assignment to unused variable
post_response(F841)
Ashutosh619-sudo
had a problem deploying
to
CI Environment
GitHub Actions
Failure
Ashutosh619-sudo
added
deploy
Ashutosh619-sudo
had a problem deploying
to
CI Environment
GitHub Actions
Failure
Ashutosh619-sudo
temporarily deployed
to
Staging
GitHub Actions
Inactive
|
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests
Chores