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]: Raise NoResultFound Exception for Missing Base #2035

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

aybruhm
Copy link
Member

@aybruhm aybruhm commented Aug 29, 2024

Description

This PR addresses an issue in the backend where attempting to fetch a configuration with a non-existent base_id resulted in a non-descriptive error. The agenta.get_config function has been updated to raise a NoResultFound exception when the specified base does not exist, ensuring that error handling is more explicit and informative.

Related Issue

Closes AGE-702

Changes Included

  • Updated db_manager fetch_base_by_id function to raise a NoResultFound exception if the provided base_id does not match any existing base.

What to QA

  • Test fetching a configuration with a valid base_id to ensure it works as expected.
  • Test fetching a configuration with an invalid or non-existent base_id and verify that the NoResultFound exception is raised and logged correctly.

Acceptance Criteria

  • The NoResultFound exception is raised when a non-existent base_id is provided.
  • The correct exception message is logged, providing clear feedback for troubleshooting.

Copy link

vercel bot commented Aug 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
agenta ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 10, 2024 10:00am
agenta-documentation ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 10, 2024 10:00am

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Aug 29, 2024
@aybruhm aybruhm requested a review from jp-agenta August 29, 2024 11:56
@dosubot dosubot bot added Backend bug Something isn't working labels Aug 29, 2024
Copy link
Member

@mmabrouk mmabrouk left a comment

Choose a reason for hiding this comment

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

The function fetch_base_by_id is called in a number of functions outside of try-except. This change might introduce unexpected bugs. Please make sure to add exception handling to wherever this function is used and make sure that the logic is not changed (for instance, please make sure that none of the places calling this function consider a None result as if there is no results).

@dosubot dosubot bot removed the size:XS This PR changes 0-9 lines, ignoring generated files. label Sep 10, 2024
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Sep 10, 2024
@aybruhm
Copy link
Member Author

aybruhm commented Sep 10, 2024

The function fetch_base_by_id is called in a number of functions outside of try-except. This change might introduce unexpected bugs. Please make sure to add exception handling to wherever this function is used and make sure that the logic is not changed (for instance, please make sure that none of the places calling this function consider a None result as if there is no results).

Actually, no. The only function where fetch_base_by_id is called without a try-except block is the construct_app_container router, which has been updated. Every other function that calls fetch_base_by_id, in one way or another, is inside a try-except block.

Functions/routers that make use of the fetch_base_by_id function and have a try-except block include:

  • save_config router
  • get_config router
  • construct_app_container_url router (this was the only place that did not have the function in a try-except block)
  • add_variant_from_base_and_config router
  • update_variant_image and terminate_app_and_remove_app_variant functions, which are used in routers that have try-except blocks

re: @mmabrouk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants