Skip to content
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

fix: Update sessions with given IDs only #290

Merged
merged 1 commit into from
Apr 22, 2024
Merged

Conversation

whiterabbit1983
Copy link
Contributor

@whiterabbit1983 whiterabbit1983 commented Apr 22, 2024

🚀 This description was created by Ellipsis for commit 6a75538

Summary:

This PR modifies the session update queries in patch_session.py and update_session.py to ensure updates are applied only to sessions with specified IDs, and adds a print statement for debugging.

Key points:

  • Modified patch_session_query and update_session_query functions in /agents-api/agents_api/models/session/patch_session.py and /agents-api/agents_api/models/session/update_session.py respectively.
  • Added ids[session_id, developer_id] to the session update query to ensure updates are applied only to sessions with given IDs.
  • Added a print statement in update_session_query for debugging.

Generated with ❤️ by ellipsis.dev

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested.

  • Reviewed the entire pull request up to a71ad5a
  • Looked at 43 lines of code in 2 files
  • Took 56 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 0 additional comments because they didn't meet confidence threshold of 50%.

Workflow ID: wflow_xEq49s9T03mJvtQO


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. We'll respond in a few minutes. Learn more here.

agents-api/agents_api/models/session/update_session.py Outdated Show resolved Hide resolved
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me!

  • Performed an incremental review on 6a75538
  • Looked at 33 lines of code in 2 files
  • Took 1 minute and 18 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 50%.
1. /agents-api/agents_api/models/session/patch_session.py:69:
  • Assessed confidence : 66%
  • Grade: 0%
  • Comment:
    The changes look good, but it would be beneficial to add tests to verify the correctness of this change. This will help ensure that the new code works as expected and doesn't introduce any regressions.
  • Reasoning:
    The PR author has added a new line to the session update query to ensure that updates are only applied to sessions with the given IDs. This is a good practice as it prevents accidental updates to other sessions. However, the author has not provided any tests to verify the correctness of this change. It's important to have tests to ensure that the new code works as expected and doesn't introduce any regressions.

Workflow ID: wflow_BUxJEImy3HDfM0aI


Not what you expected? You can customize the content of the reviews using rules. Learn more here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants