Move all the files related to the OpenSearch service to the existing OpenSearch folder#406
Conversation
…OpenSearch folder
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
React App | Jest test suite - Code coverage reportTotal: 26.41%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Dedupe Hub App | Unittest test suite - Code coverage reportTotal: 56.14%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Contricleaner App | Unittest test suite - Code coverage reportTotal: 98.91%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
WalkthroughThe changes in this pull request involve significant updates to the Open Supply Hub project, particularly in the organization and functionality of the OpenSearch service. Key modifications include the relocation of OpenSearch-related files, an update to OpenSearch version 2.15, and the introduction of a new Logstash pipeline for moderation event data. Additionally, the release notes for version 1.24.0 have been updated to reflect these changes, and various import paths have been adjusted throughout the codebase to align with the new module structure. Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (9)
src/django/api/services/opensearch/search.py (5)
Line range hint
9-15: Consider enhancing error message context.While the custom exception class is a good practice, consider accepting additional context parameters to make error messages more specific and helpful for debugging.
Here's a suggested enhancement:
class OpenSearchServiceException(Exception): def __init__( self, - message="An unexpected error occurred while processing the request." + message="An unexpected error occurred while processing the request.", + details=None ): self.message = message + self.details = details - super().__init__(self.message) + super().__init__(f"{self.message} Details: {self.details}" if details else self.message)
Line range hint
18-19: Consider adding type hints and validation for the client parameter.The constructor could benefit from type hints and validation to ensure the client meets the expected interface.
- def __init__(self, client=None): + def __init__(self, client: Optional[OpenSearchClient] = None) -> None: self.__client = client or OpenSearchServiceConnection().client + if not hasattr(self.__client, 'search'): + raise ValueError("Invalid OpenSearch client provided")
Line range hint
21-27: Strengthen the field renaming logic.The current implementation could be more robust by handling nested dictionaries more safely and validating data types.
def __rename_lon_field(self, source): + if not isinstance(source, dict): + return source coordinates = source.get('coordinates') - if not coordinates: + if not isinstance(coordinates, dict): return source - else: - coordinates['lng'] = coordinates.pop("lon") - return source + if 'lon' in coordinates: + coordinates['lng'] = coordinates.pop('lon') + return source
Line range hint
29-46: Enhance error handling and add type hints in response preparation.The response preparation method could benefit from more specific error handling and type annotations.
- def __prepare_opensearch_response(self, response): + def __prepare_opensearch_response(self, response: dict) -> dict: if not response or "hits" not in response: logger.error(f"Invalid response format: {response}") raise OpenSearchServiceException( - "Invalid response format from OpenSearch." + "Invalid response format from OpenSearch.", + details=f"Expected 'hits' in response, got: {list(response.keys()) if response else None}" ) total_hits = response.get("hits", {}).get("total", {}).get("value", 0) hits = response.get("hits", {}).get("hits", [])
Line range hint
48-61: Improve search_index method with pagination and specific error handling.The search method could benefit from pagination parameters and more specific error handling.
- def search_index(self, index_name, query_body): + def search_index(self, index_name: str, query_body: dict, size: int = 10, from_: int = 0) -> dict: try: - response = self.__client.search(body=query_body, index=index_name) + response = self.__client.search( + body=query_body, + index=index_name, + size=size, + from_=from_ + ) return self.__prepare_opensearch_response(response) except OpenSearchException as e: logger.error(f"An error occurred while searching in \ index '{index_name}': {e}") - raise OpenSearchServiceException() + raise OpenSearchServiceException( + "Failed to execute OpenSearch query", + details=str(e) + ) except Exception as e: logger.error(f"An unexpected error occurred: {e}") - raise OpenSearchServiceException() + raise OpenSearchServiceException( + "Unexpected error during search operation", + details=str(e) + )src/django/api/tests/test_opensearch_relevant_data.py (2)
Line range hint
43-83: Consider updating the test data timestamps.While the test logic is sound, the hardcoded timestamp
'2019-03-24 02:23:22.195 +0100'is quite old. Consider using a more recent timestamp or a dynamic one relative to the current time for more realistic test scenarios.- self.old_updated_at = '2019-03-24 02:23:22.195 +0100' + self.old_updated_at = (timezone.now() - timezone.timedelta(days=1)).isoformat()
Line range hint
85-86: Update the logger import path.The mock patch still references the old path
api.services.search.logger. This should be updated to match the new module structure.- @patch('api.services.search.logger') + @patch('api.services.opensearch.search.logger')src/django/api/tests/test_opensearch_response_formatter.py (1)
Import path mismatch in test file needs attention
The test file imports from
api.services.opensearch.searchbut uses@patch('api.services.search.logger')in its test methods. This inconsistency needs to be fixed by updating the logger patch path to match the new module location.
- Update all
@patch('api.services.search.logger')decorators to@patch('api.services.opensearch.search.logger')insrc/django/api/tests/test_opensearch_response_formatter.py🔗 Analysis chain
Line range hint
1-150: Test structure and coverage look goodThe test suite is well-structured and provides comprehensive coverage of the OpenSearch response formatting functionality. The relocation of the OpenSearch service to its dedicated folder is a good organizational improvement.
Let's verify that the OpenSearch service has been properly moved to its new location:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the OpenSearch service file structure # Test 1: Verify the new location exists fd --type f "search.py" src/django/api/services/opensearch/ # Test 2: Verify no OpenSearch-related files remain in the old location fd --type f "search.py" src/django/api/services/ --exclude opensearch/Length of output: 224
Script:
#!/bin/bash # Let's verify the imports in the test file match the actual module locations # Check the actual imports in the test file rg "from api\.services.*import" src/django/api/tests/test_opensearch_response_formatter.py # Check the logger import path in the actual service files rg "import.*logger" src/django/api/services/opensearch/search.pyLength of output: 204
doc/release/RELEASE-NOTES.md (1)
Line range hint
1-3000: Consider removing empty sections for better readabilityThe release notes contain empty sections marked with asterisks (e.g., "Describe scheme changes here."). Consider removing these empty sections to improve readability while maintaining the structured format.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
doc/release/RELEASE-NOTES.md(1 hunks)src/django/api/services/opensearch/search.py(1 hunks)src/django/api/signals.py(1 hunks)src/django/api/tests/test_opensearch_relevant_data.py(1 hunks)src/django/api/tests/test_opensearch_response_formatter.py(1 hunks)src/django/api/tests/test_v1_utils.py(1 hunks)src/django/api/views/v1/production_locations.py(1 hunks)src/django/api/views/v1/utils.py(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/django/api/signals.py
- src/django/api/tests/test_v1_utils.py
- src/django/api/views/v1/production_locations.py
🔇 Additional comments (5)
src/django/api/services/opensearch/search.py (2)
1-6: LGTM! Clean import structure and proper logging setup.
The import statements correctly reflect the new module structure, and the logging setup follows best practices.
Line range hint 18-61: Verify consistency with other OpenSearch implementations.
Let's check for similar OpenSearch service implementations to ensure consistent patterns across the codebase.
✅ Verification successful
Let me gather more specific information about the OpenSearch implementations.
Based on the gathered information, I can now provide the final response:
OpenSearch implementation follows consistent patterns and best practices
The implementation is well-structured and follows consistent patterns across the codebase:
- The connection handling is properly centralized in
OpenSearchServiceConnectionsingleton class - Response formatting is standardized and well-tested as evidenced by comprehensive test coverage
- The coordinate field renaming (lon → lng) is unique to this service and doesn't conflict with other implementations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other OpenSearch service implementations
echo "Searching for other OpenSearch service implementations..."
rg -l "OpenSearchService" --type py
# Search for similar search implementations
echo "Searching for similar search patterns..."
ast-grep --pattern 'def search_index($$$)'
Length of output: 2479
Script:
#!/bin/bash
# Check the content of the main OpenSearch service file
echo "Content of opensearch.py:"
cat src/django/api/services/opensearch/opensearch.py
# Look for similar response formatting patterns
echo -e "\nSearching for response formatting patterns:"
rg -A 5 "prepare.*response" --type py
# Look for coordinate field handling
echo -e "\nSearching for coordinate field handling:"
rg -A 3 "coordinates\[.(lon|lng)" --type py
Length of output: 10894
src/django/api/tests/test_opensearch_relevant_data.py (1)
4-5: LGTM! Import path updated correctly.
The import statement has been properly updated to reflect the new module structure, maintaining consistency with the OpenSearch service reorganization.
Let's verify the consistency of import paths across the codebase:
✅ Verification successful
Import path update verified and consistent across the codebase
The verification confirms that:
- All OpenSearchService imports now use the new path
api.services.opensearch.search - No instances of the old import path
api.services.searchremain - The class definitions and related imports are properly organized in the new opensearch directory
- All dependent files are using the correct import paths
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all imports of OpenSearchService use the new path
# Test 1: Check for any remaining old imports
# Expect: No matches as all should be updated
echo "Checking for old imports..."
rg "from api.services.search import OpenSearchService"
# Test 2: Verify all imports use the new path
# Expect: All OpenSearchService imports should use the new path
echo "Verifying new imports..."
rg "from api.services.opensearch.search import OpenSearchService"
Length of output: 568
Script:
#!/bin/bash
# Let's verify if there are any other variations of OpenSearchService imports
# or usages that might need attention
# Check for any OpenSearchService class definitions or references
echo "Checking for OpenSearchService class definitions and references..."
rg "class OpenSearchService" -A 2
# Check for any remaining imports with 'search' in the path
echo "Checking for other search-related imports..."
rg "from api.services.search import"
# Check for any imports containing OpenSearch
echo "Checking for all OpenSearch-related imports..."
rg "from.*import.*OpenSearch"
Length of output: 2526
src/django/api/views/v1/utils.py (1)
6-6: LGTM! Import path update aligns with module restructuring.
The updated import path correctly reflects the new organizational structure where OpenSearch-related files are consolidated in the opensearch folder.
Let's verify that this import path is consistently updated across the codebase:
✅ Verification successful
Import path update is consistently used across the codebase
The verification confirms that:
- No instances of the old import path (
api.services.search) remain - The new import path (
api.services.opensearch.search) is consistently used in both the main implementation and test files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all imports of OpenSearchServiceException use the new path
# Test: Search for any remaining old imports. Expect: No results
rg "from api.services.search import OpenSearchServiceException"
# Test: Verify new import path is used consistently. Expect: All imports use new path
rg "from api.services.opensearch.search import OpenSearchServiceException"
Length of output: 353
doc/release/RELEASE-NOTES.md (1)
Line range hint 1-3000: Documentation quality meets high standards
The release notes demonstrate:
- Consistent JIRA ticket references for traceability
- Clear technical explanations with rationale
- Actionable release instructions
- Well-structured format across releases
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
vladsha-dev
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
Django App | Unittest test suite - Code coverage reportTotal: 77.91%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
vladsha-dev
deleted the
move-all-the-files-related-to-the-opensearch-service-to-the-existing-opensearch-folder
branch
…OpenSearch folder (#406) Moved all the files related to the OpenSearch service to the existing `src/django/api/services/opensearch` folder within the `api` app of the Django application. This should make it easier to navigate through the files and clarify the location of all OpenSearch service-related files in one place within the `api` app in Django.
3 participants
Moved all the files related to the OpenSearch service to the existing
src/django/api/services/opensearchfolder within theapiapp of the Django application. This should make it easier to navigate through the files and clarify the location of all OpenSearch service-related files in one place within theapiapp in Django.