-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hotfix/v0.7 jira fix #2722
Hotfix/v0.7 jira fix #2722
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR implements a hotfix for version 0.7, focusing on the Jira connector and related improvements. Key changes include:
- Modified Jira connector to handle reserved words in project names by quoting the project name in JQL queries
- Added a new test file
test_jira_basic.py
to verify basic Jira connector functionality - Introduced
delete_single
method inVespaIndex
class for more efficient single document deletion - Updated Redis and Celery configurations to enhance connection reliability and performance
- Improved log management in
supervisord.conf
, consolidating logs and introducing environment variable-based configuration - Added
CELERY_BROKER_POOL_LIMIT
environment variable to Docker Compose files for better Celery broker pool control
13 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings
start_ind = 0 | ||
while True: | ||
doc_batch, fetched_batch_size = fetch_jira_issues_batch( | ||
jql=f"project = {self.jira_project}", | ||
jql=f"project = {quoted_project}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Use an f-string for consistency with other string formatting in the file
@@ -276,8 +278,10 @@ def poll_source( | |||
"%Y-%m-%d %H:%M" | |||
) | |||
|
|||
# Quote the project name to handle reserved words | |||
quoted_project = f'"{self.jira_project}"' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Duplicate code from line 249. Consider creating a method to quote project names
f"project = {quoted_project} AND " | ||
f"updated >= '{start_date_str}' AND " | ||
f"updated <= '{end_date_str}'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Use triple quotes for multi-line strings to improve readability
|
||
|
||
def test_jira_connector_basic(jira_connector: JiraConnector) -> None: | ||
doc_batch_generator = jira_connector.poll_source(0, time.time()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Using 0 as the start time may not be ideal. Consider using a more recent timestamp to ensure relevant data is fetched.
with pytest.raises(StopIteration): | ||
next(doc_batch_generator) | ||
|
||
assert len(doc_batch) == 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Asserting exactly one document may be too rigid. Consider allowing a range of documents or parameterizing this test.
assert doc.id == "https://danswerai.atlassian.net/browse/AS-2" | ||
assert doc.semantic_identifier == "test123small" | ||
assert doc.source == DocumentSource.JIRA | ||
assert doc.metadata == {"priority": "Medium", "status": "Backlog"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: These assertions are tightly coupled to specific Jira issue data. Consider making these checks more flexible or use mock data.
|
||
assert len(doc.sections) == 1 | ||
section = doc.sections[0] | ||
assert section.text == "example_text\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: The expected text 'example_text\n' is very specific. Consider using a more general assertion or regex pattern.
^