updated readme of files

This commit is contained in:
tushar 2025-09-29 20:06:54 +05:30
parent a0c205ef63
commit cb8b6d04f5
6 changed files with 97 additions and 425 deletions

76
COMMENT_CLEANUP.md Normal file
View file

@ -0,0 +1,76 @@
# Comment Cleanup Summary
## Removed Problematic Comments
### 1. **Vague "Optionally" Comments**
**Removed from:** `surfsense_backend/app/routes/search_source_connectors_routes.py`
- Removed all instances of `# Optionally update status in DB to indicate failure`
- These comments were vague and didn't provide actionable information
### 2. **Redundant Explanatory Comments**
**Removed from:** `surfsense_backend/app/tasks/connector_indexers/github_indexer.py`
- Removed `# Update the last_indexed_at timestamp for the connector only if requested`
- Removed `# Commit all changes at the end`
- Removed `# Log success`
**Removed from:** `surfsense_backend/app/routes/search_source_connectors_routes.py`
- Removed `# Don't update timestamp in the indexing function` (multiple instances)
- Removed `# Note: This assumes 'name' and 'is_indexable' are not crucial for config validation itself`
- Removed inline comments like `# Use existing name`, `# Not used by validator`
### 3. **Verbose Docstring Notes**
**Cleaned up in:** `surfsense_backend/app/tasks/connector_indexers/github_indexer.py`
- Removed redundant notes from parameter descriptions
- Simplified docstring to be more concise
### 4. **Debug Files**
**Removed:**
- `debug_github_connector.py`
- `validate_github_fix.py`
- `final_validation.py`
## Code Quality Improvements
### ✅ **Before Cleanup:**
```python
# Optionally update status in DB to indicate failure
update_last_indexed=False, # Don't update timestamp in the indexing function
# Update the last_indexed_at timestamp for the connector only if requested
if update_last_indexed:
await update_connector_last_indexed(session, connector, update_last_indexed)
# Commit all changes at the end
await session.commit()
# Log success
await task_logger.log_task_success(...)
```
### ✅ **After Cleanup:**
```python
update_last_indexed=False,
if update_last_indexed:
await update_connector_last_indexed(session, connector, update_last_indexed)
await session.commit()
await task_logger.log_task_success(...)
```
## Benefits
1. **Cleaner Code**: Removed unnecessary noise that didn't add value
2. **Better Readability**: Code is now more concise and easier to read
3. **Professional Appearance**: Removed vague/tentative language like "optionally"
4. **Reduced Maintenance**: Fewer comments to maintain and update
5. **Code Review Friendly**: Eliminates potential nitpick comments about redundant documentation
## What Was Preserved
- **Functional comments**: Comments that explain complex business logic
- **Important documentation**: Docstrings and API documentation
- **Contextual comments**: Comments that provide necessary context for understanding code flow
The codebase now follows clean code principles with comments that add genuine value rather than restating what the code obviously does.

View file

@ -1,127 +0,0 @@
#!/usr/bin/env python3
"""
Debug script to test GitHub connector indexing issues.
This script can help diagnose GitHub connector indexing problems by:
1. Checking if documents are being created in the database
2. Verifying last_indexed_at timestamp updates
3. Testing document retrieval functionality
Usage:
python debug_github_connector.py --connector-id <id> --search-space-id <id>
"""
import asyncio
import argparse
import sys
import os
from datetime import datetime
# Add the backend directory to Python path
backend_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'surfsense_backend')
sys.path.insert(0, backend_dir)
async def debug_github_connector(connector_id: int, search_space_id: int):
"""Debug function to test GitHub connector."""
try:
# Import modules
from app.db import get_async_session, Document, SearchSourceConnector
from sqlalchemy.future import select
print(f"🔍 Debugging GitHub Connector {connector_id} in Search Space {search_space_id}")
print("=" * 60)
# Get a database session
async with get_async_session().__aenter__() as session:
# 1. Check if connector exists
print("1. Checking connector existence...")
result = await session.execute(
select(SearchSourceConnector).filter(SearchSourceConnector.id == connector_id)
)
connector = result.scalars().first()
if not connector:
print(f"❌ Connector {connector_id} not found!")
return
print(f"✅ Connector found: {connector.name}")
print(f" Type: {connector.connector_type}")
print(f" Last indexed: {connector.last_indexed_at}")
print()
# 2. Check documents count
print("2. Checking documents count...")
result = await session.execute(
select(Document).filter(
Document.search_space_id == search_space_id,
Document.document_type == 'GITHUB_CONNECTOR'
)
)
documents = result.scalars().all()
print(f"📄 Found {len(documents)} GitHub documents in search space {search_space_id}")
if documents:
print(" Recent documents:")
for i, doc in enumerate(documents[:5]): # Show first 5
print(f" - {doc.title} (ID: {doc.id}, Created: {doc.created_at})")
print()
# 3. Check all documents in search space
print("3. Checking all documents in search space...")
result = await session.execute(
select(Document).filter(Document.search_space_id == search_space_id)
)
all_documents = result.scalars().all()
print(f"📄 Total documents in search space {search_space_id}: {len(all_documents)}")
if all_documents:
# Group by document type
doc_types = {}
for doc in all_documents:
doc_type = doc.document_type.value if hasattr(doc.document_type, 'value') else str(doc.document_type)
doc_types[doc_type] = doc_types.get(doc_type, 0) + 1
print(" Documents by type:")
for doc_type, count in doc_types.items():
print(f" - {doc_type}: {count}")
print()
# 4. Test document retrieval query (simulate API call)
print("4. Testing document retrieval API simulation...")
from app.routes.documents_routes import read_documents
from app.users import current_active_user
# This is just a simulation - in real usage, you'd need proper auth
print(" (Note: This would require proper authentication in real usage)")
print(" The documents should be accessible via: GET /api/v1/documents?search_space_id={search_space_id}")
except Exception as e:
print(f"❌ Error during debugging: {e}")
import traceback
traceback.print_exc()
def main():
parser = argparse.ArgumentParser(description='Debug GitHub connector indexing issues')
parser.add_argument('--connector-id', type=int, required=True, help='GitHub connector ID')
parser.add_argument('--search-space-id', type=int, required=True, help='Search space ID')
args = parser.parse_args()
print("GitHub Connector Debug Script")
print("============================")
print(f"Connector ID: {args.connector_id}")
print(f"Search Space ID: {args.search_space_id}")
print()
# Run the debug function
try:
asyncio.run(debug_github_connector(args.connector_id, args.search_space_id))
except KeyboardInterrupt:
print("❌ Script interrupted by user")
except Exception as e:
print(f"❌ Script failed: {e}")
import traceback
traceback.print_exc()
if __name__ == "__main__":
main()

View file

@ -1,154 +0,0 @@
"""
Final validation test for the GitHub connector fixes.
This ensures the fixes are consistent with the existing codebase patterns.
"""
def test_route_pattern_consistency():
"""Test that our GitHub route follows the same pattern as other routes."""
print("🔧 Route Pattern Consistency Test:")
print(" Standard pattern for all connectors:")
print(" 1. Call indexer with update_last_indexed=False")
print(" 2. If error_message exists, log error and don't update timestamp")
print(" 3. If no error, manually update timestamp via route function")
print(" 4. Commit the timestamp update")
print()
# Simulate GitHub route logic
def simulate_github_route():
# Step 1: Call indexer (simulated)
indexed_count = 5
error_message = None # Simulate success
print(f" Step 1: indexer returns ({indexed_count}, {error_message})")
# Step 2 & 3: Route logic
if error_message:
print(" Step 2: Error detected - would log error, no timestamp update")
return False
else:
print(" Step 3: Success - would call update_connector_last_indexed()")
print(" Step 4: Would commit timestamp update")
return True
success = simulate_github_route()
if success:
print(" ✅ GitHub route follows standard pattern")
else:
print(" ❌ GitHub route does not follow standard pattern")
return success
def test_indexer_consistency():
"""Test that the GitHub indexer follows the same pattern as other indexers."""
print("\n🔧 Indexer Pattern Consistency Test:")
print(" Standard pattern for all indexers:")
print(" 1. Accept update_last_indexed parameter (default True)")
print(" 2. If update_last_indexed=True, call update_connector_last_indexed()")
print(" 3. Commit all changes including documents and timestamp")
print(" 4. Return (documents_processed, error_message)")
print()
# Simulate GitHub indexer logic
def simulate_github_indexer(update_last_indexed=True):
documents_processed = 5
errors = []
print(f" Processing documents... processed {documents_processed}")
# The key fix: timestamp update logic
if update_last_indexed:
print(" update_last_indexed=True: calling update_connector_last_indexed()")
else:
print(" update_last_indexed=False: skipping timestamp update")
print(" Committing all changes (documents + timestamp if applicable)")
# Return logic
error_message = "; ".join(errors) if errors else None
print(f" Returning: ({documents_processed}, {error_message})")
return documents_processed, error_message
# Test both scenarios
print(" Scenario 1 - Route calls with update_last_indexed=False:")
result1 = simulate_github_indexer(update_last_indexed=False)
print("\n Scenario 2 - Direct call with update_last_indexed=True:")
result2 = simulate_github_indexer(update_last_indexed=True)
print(" ✅ GitHub indexer handles both scenarios correctly")
return True
def test_timezone_consistency():
"""Test that timezone handling is consistent across the codebase."""
print("\n🕐 Timezone Consistency Test:")
from datetime import UTC, datetime
# Test the pattern we've implemented
timestamp = datetime.now(UTC)
print(f" Generated timestamp: {timestamp}")
print(f" Timezone info: {timestamp.tzinfo}")
print(f" Is timezone aware: {timestamp.tzinfo is not None}")
# Verify it matches database expectations
print(" Database schema expects: TIMESTAMP(timezone=True)")
print(" Our timestamp provides timezone info: ✅")
return True
def validate_complete_fix():
"""Validate that all components work together."""
print("\n🎯 Complete Fix Validation:")
print(" The complete flow:")
print(" 1. Route calls GitHub indexer with update_last_indexed=False")
print(" 2. GitHub indexer processes documents but skips timestamp")
print(" 3. GitHub indexer commits documents and returns success")
print(" 4. Route sees success, calls its own timestamp update function")
print(" 5. Route commits timestamp update")
print(" 6. Documents appear in UI, timestamp persists")
print("\n Key fixes applied:")
print(" ✅ Added missing update_connector_last_indexed import/call")
print(" ✅ Fixed timezone handling (UTC instead of naive)")
print(" ✅ Made route pattern consistent with other connectors")
print(" ✅ Proper session/transaction management")
return True
if __name__ == "__main__":
print("GitHub Connector Fix - Final Validation")
print("=" * 50)
try:
all_tests_passed = True
all_tests_passed &= test_route_pattern_consistency()
all_tests_passed &= test_indexer_consistency()
all_tests_passed &= test_timezone_consistency()
all_tests_passed &= validate_complete_fix()
if all_tests_passed:
print("\n🎉 All validations passed! The project looks good.")
print("\nWhat was fixed:")
print("• Documents will now persist in the database")
print("• Last indexed timestamps will stick after page refresh")
print("• Queries will return results from indexed repositories")
print("• GitHub connector now follows the same pattern as others")
print("\nNext steps:")
print("1. Test with a real GitHub repository")
print("2. Verify documents appear in 'Manage Documents'")
print("3. Confirm queries return relevant results")
else:
print("❌ Some validations failed!")
except Exception as e:
print(f"❌ Validation failed: {e}")
import traceback
traceback.print_exc()

View file

@ -230,13 +230,11 @@ async def update_search_source_connector(
)
try:
# We can reuse the base validator by creating a temporary base model instance
# Note: This assumes 'name' and 'is_indexable' are not crucial for config validation itself
temp_data_for_validation = {
"name": db_connector.name, # Use existing name
"name": db_connector.name,
"connector_type": current_connector_type,
"is_indexable": db_connector.is_indexable, # Use existing value
"last_indexed_at": db_connector.last_indexed_at, # Not used by validator
"is_indexable": db_connector.is_indexable,
"last_indexed_at": db_connector.last_indexed_at,
"config": merged_config,
}
SearchSourceConnectorBase.model_validate(temp_data_for_validation)
@ -669,7 +667,7 @@ async def run_slack_indexing(
user_id=user_id,
start_date=start_date,
end_date=end_date,
update_last_indexed=False, # Don't update timestamp in the indexing function
update_last_indexed=False,
)
# Only update last_indexed_at if indexing was successful (either new docs or updated docs)
@ -731,7 +729,7 @@ async def run_notion_indexing(
user_id=user_id,
start_date=start_date,
end_date=end_date,
update_last_indexed=False, # Don't update timestamp in the indexing function
update_last_indexed=False,
)
# Only update last_indexed_at if indexing was successful (either new docs or updated docs)
@ -784,13 +782,12 @@ async def run_github_indexing(
user_id,
start_date,
end_date,
update_last_indexed=False, # Don't update timestamp in the indexing function
update_last_indexed=False,
)
if error_message:
logger.error(
f"GitHub indexing failed for connector {connector_id}: {error_message}"
)
# Optionally update status in DB to indicate failure
else:
logger.info(
f"GitHub indexing successful for connector {connector_id}. Indexed {indexed_count} documents."
@ -804,7 +801,6 @@ async def run_github_indexing(
f"Critical error in run_github_indexing for connector {connector_id}: {e}",
exc_info=True,
)
# Optionally update status in DB to indicate failure
# Add new helper functions for Linear indexing
@ -849,7 +845,6 @@ async def run_linear_indexing(
logger.error(
f"Linear indexing failed for connector {connector_id}: {error_message}"
)
# Optionally update status in DB to indicate failure
else:
logger.info(
f"Linear indexing successful for connector {connector_id}. Indexed {indexed_count} documents."
@ -863,7 +858,7 @@ async def run_linear_indexing(
f"Critical error in run_linear_indexing for connector {connector_id}: {e}",
exc_info=True,
)
# Optionally update status in DB to indicate failure
# Add new helper functions for discord indexing
@ -970,7 +965,7 @@ async def run_jira_indexing(
logger.error(
f"Jira indexing failed for connector {connector_id}: {error_message}"
)
# Optionally update status in DB to indicate failure
else:
logger.info(
f"Jira indexing successful for connector {connector_id}. Indexed {indexed_count} documents."
@ -983,7 +978,7 @@ async def run_jira_indexing(
f"Critical error in run_jira_indexing for connector {connector_id}: {e}",
exc_info=True,
)
# Optionally update status in DB to indicate failure
# Add new helper functions for Confluence indexing
@ -1030,7 +1025,7 @@ async def run_confluence_indexing(
logger.error(
f"Confluence indexing failed for connector {connector_id}: {error_message}"
)
# Optionally update status in DB to indicate failure
else:
logger.info(
f"Confluence indexing successful for connector {connector_id}. Indexed {indexed_count} documents."
@ -1043,7 +1038,7 @@ async def run_confluence_indexing(
f"Critical error in run_confluence_indexing for connector {connector_id}: {e}",
exc_info=True,
)
# Optionally update status in DB to indicate failure
# Add new helper functions for ClickUp indexing
@ -1088,7 +1083,7 @@ async def run_clickup_indexing(
logger.error(
f"ClickUp indexing failed for connector {connector_id}: {error_message}"
)
# Optionally update status in DB to indicate failure
else:
logger.info(
f"ClickUp indexing successful for connector {connector_id}. Indexed {indexed_count} tasks."
@ -1101,7 +1096,7 @@ async def run_clickup_indexing(
f"Critical error in run_clickup_indexing for connector {connector_id}: {e}",
exc_info=True,
)
# Optionally update status in DB to indicate failure
# Add new helper functions for Airtable indexing
@ -1146,7 +1141,7 @@ async def run_airtable_indexing(
logger.error(
f"Airtable indexing failed for connector {connector_id}: {error_message}"
)
# Optionally update status in DB to indicate failure
else:
logger.info(
f"Airtable indexing successful for connector {connector_id}. Indexed {indexed_count} records."
@ -1159,7 +1154,7 @@ async def run_airtable_indexing(
f"Critical error in run_airtable_indexing for connector {connector_id}: {e}",
exc_info=True,
)
# Optionally update status in DB to indicate failure
# Add new helper functions for Google Calendar indexing
@ -1206,7 +1201,7 @@ async def run_google_calendar_indexing(
logger.error(
f"Google Calendar indexing failed for connector {connector_id}: {error_message}"
)
# Optionally update status in DB to indicate failure
else:
logger.info(
f"Google Calendar indexing successful for connector {connector_id}. Indexed {indexed_count} documents."
@ -1219,7 +1214,7 @@ async def run_google_calendar_indexing(
f"Critical error in run_google_calendar_indexing for connector {connector_id}: {e}",
exc_info=True,
)
# Optionally update status in DB to indicate failure
async def run_google_gmail_indexing_with_new_session(
@ -1265,7 +1260,7 @@ async def run_google_gmail_indexing(
logger.error(
f"Google Gmail indexing failed for connector {connector_id}: {error_message}"
)
# Optionally update status in DB to indicate failure
else:
logger.info(
f"Google Gmail indexing successful for connector {connector_id}. Indexed {indexed_count} documents."
@ -1278,7 +1273,7 @@ async def run_google_gmail_indexing(
f"Critical error in run_google_gmail_indexing for connector {connector_id}: {e}",
exc_info=True,
)
# Optionally update status in DB to indicate failure
# Add new helper functions for luma indexing

View file

@ -43,8 +43,8 @@ async def index_github_repos(
connector_id: ID of the GitHub connector
search_space_id: ID of the search space to store documents in
user_id: ID of the user
start_date: Start date for filtering (YYYY-MM-DD format) - Note: GitHub indexing processes all files regardless of dates
end_date: End date for filtering (YYYY-MM-DD format) - Note: GitHub indexing processes all files regardless of dates
start_date: Start date for filtering (YYYY-MM-DD format)
end_date: End date for filtering (YYYY-MM-DD format)
update_last_indexed: Whether to update the last_indexed_at timestamp (default: True)
Returns:
@ -302,17 +302,14 @@ async def index_github_repos(
)
errors.append(f"Failed processing {repo_full_name}: {repo_err}")
# Update the last_indexed_at timestamp for the connector only if requested
if update_last_indexed:
await update_connector_last_indexed(session, connector, update_last_indexed)
# Commit all changes at the end
await session.commit()
logger.info(
f"Finished GitHub indexing for connector {connector_id}. Processed {documents_processed} files."
)
# Log success
await task_logger.log_task_success(
log_entry,
f"Successfully completed GitHub indexing for connector {connector_id}",

View file

@ -1,115 +0,0 @@
"""
Quick validation test for the GitHub connector fixes.
This script validates the logic without requiring a full database setup.
"""
from datetime import UTC, datetime
def test_timezone_aware_timestamp():
"""Test that we're creating timezone-aware timestamps correctly."""
now_utc = datetime.now(UTC)
now_naive = datetime.now()
print("🕐 Timezone Test:")
print(f" UTC timestamp: {now_utc} (timezone: {now_utc.tzinfo})")
print(f" Naive timestamp: {now_naive} (timezone: {now_naive.tzinfo})")
# The UTC version should have timezone info
assert now_utc.tzinfo is not None, "UTC timestamp should have timezone info"
print(" ✅ UTC timestamp correctly has timezone info")
# The naive version should not
assert now_naive.tzinfo is None, "Naive timestamp should not have timezone info"
print(" ✅ Naive timestamp correctly has no timezone info")
print(" ✅ Timezone handling is correct!\n")
def test_update_logic_simulation():
"""Simulate the update_connector_last_indexed logic."""
class MockConnector:
def __init__(self):
self.last_indexed_at = None
class MockLogger:
def info(self, msg):
print(f" LOG: {msg}")
# Simulate the function
def update_connector_last_indexed_mock(connector, update_last_indexed=True):
logger = MockLogger()
if update_last_indexed:
connector.last_indexed_at = datetime.now(UTC)
logger.info(f"Updated last_indexed_at to {connector.last_indexed_at}")
print("📝 Update Logic Test:")
connector = MockConnector()
# Test with update_last_indexed=True
print(" Testing with update_last_indexed=True:")
update_connector_last_indexed_mock(connector, True)
assert connector.last_indexed_at is not None, "Should have updated timestamp"
assert connector.last_indexed_at.tzinfo is not None, "Should be timezone-aware"
print(" ✅ Timestamp updated correctly")
# Test with update_last_indexed=False
print(" Testing with update_last_indexed=False:")
original_time = connector.last_indexed_at
update_connector_last_indexed_mock(connector, False)
assert connector.last_indexed_at == original_time, "Should not have changed timestamp"
print(" ✅ Timestamp preserved when update_last_indexed=False")
print(" ✅ Update logic is correct!\n")
def test_indexer_flow_simulation():
"""Simulate the full GitHub indexer flow."""
print("🔄 Indexer Flow Test:")
# Simulate successful indexing
documents_processed = 5
errors = []
update_last_indexed = True
print(f" Simulating indexing: processed {documents_processed} documents")
print(f" Errors: {len(errors)}")
print(f" Update last indexed: {update_last_indexed}")
# This is the logic from our fix
if update_last_indexed:
print(" → Would call update_connector_last_indexed(session, connector, True)")
print(" → This would set connector.last_indexed_at = datetime.now(UTC)")
# Check return value logic
error_message = "; ".join(errors) if errors else None
print(f" Return values: ({documents_processed}, {error_message})")
# This simulates the route logic
if error_message:
print(" → Route would log error and not update timestamp")
else:
print(" → Route sees no error_message (None)")
print(" → Route knows indexer already handled timestamp update")
print(" ✅ Indexer flow logic is correct!\n")
if __name__ == "__main__":
print("GitHub Connector Fix Validation")
print("=" * 40)
try:
test_timezone_aware_timestamp()
test_update_logic_simulation()
test_indexer_flow_simulation()
print("🎉 All tests passed! The GitHub connector fixes look good.")
print("\nKey improvements verified:")
print("✅ Timezone-aware timestamps (UTC)")
print("✅ Proper update_last_indexed logic")
print("✅ Consistent with other indexers")
print("✅ Correct error handling flow")
except Exception as e:
print(f"❌ Test failed: {e}")
import traceback
traceback.print_exc()