Skip to content

ITEP-32484 - Implement GET project_configuration endpoint #220

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

Merged
merged 12 commits into from
May 20, 2025

Conversation

maxxgx
Copy link
Contributor

@maxxgx maxxgx commented May 15, 2025

πŸ“ Description

Add GET project_configuration endpoint

✨ Type of Change

Select the type of change your PR introduces:

  • 🐞 Bug fix – Non-breaking change which fixes an issue
  • πŸš€ New feature – Non-breaking change which adds functionality
  • πŸ”¨ Refactor – Non-breaking change which refactors the code base
  • πŸ’₯ Breaking change – Changes that break existing functionality
  • πŸ“š Documentation update
  • πŸ”’ Security update
  • πŸ§ͺ Tests

πŸ§ͺ Testing Scenarios

Describe how the changes were tested and how reviewers can test them too:

  • βœ… Tested manually
  • πŸ€– Run automated end-to-end tests

βœ… Checklist

Before submitting the PR, ensure the following:

  • πŸ” PR title is clear and descriptive
  • πŸ“ For internal contributors: If applicable, include the JIRA ticket number (e.g., ITEP-123456) in the PR title. Do not include full URLs
  • πŸ’¬ I have commented my code, especially in hard-to-understand areas
  • πŸ“„ I have made corresponding changes to the documentation
  • βœ… I have added tests that prove my fix is effective or my feature works

@maxxgx maxxgx requested a review from a team as a code owner May 15, 2025 08:32
@github-actions github-actions bot added the IAI Interactive AI backend label May 15, 2025
Comment on lines 28 to 29
# The project configuration is a Pydantic model, so we can simply convert it to a dict
return project_config.model_dump()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid you can't obtain the right response structure by only calling model_dump. According to the design, each parameter should have a "key", "name", "description", "type", "value", "min_value", "max_value", "default_value", etc... You can extract this information from the configuration Fields but you have to write some custom logic.

Copy link
Contributor Author

@maxxgx maxxgx May 16, 2025

Choose a reason for hiding this comment

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

You're right, added configuration rest view class

Edit: added configurable_parameters_to_rest which construct the REST view from any pydantic model.

@maxxgx maxxgx requested a review from leoll2 May 19, 2025 13:04
@maxxgx maxxgx requested a review from JortBergfeld May 20, 2025 07:07
@maxxgx maxxgx added this pull request to the merge queue May 20, 2025
Copy link
Contributor

@JortBergfeld JortBergfeld left a comment

Choose a reason for hiding this comment

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

LGTM, I think it should be easy enough to extend the project creation BDD with a GET project configuration step to add an e2e test for this new endpoint.

"""
project_config = ProjectConfigurationRepo(project_identifier).get_project_configuration()
if isinstance(project_config, NullProjectConfiguration):
raise ProjectNotFoundException(project_identifier.project_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps it is cleaner to create a new ProjectConfigurationNotFoundException or to use a custom message indicating that the configuration could not be found.

@JortBergfeld JortBergfeld removed this pull request from the merge queue due to a manual request May 20, 2025
@maxxgx maxxgx added this pull request to the merge queue May 20, 2025
Merged via the queue into main with commit 28cef56 May 20, 2025
10 checks passed
@maxxgx maxxgx deleted the max/get-project-configuration-endpoint branch May 20, 2025 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IAI Interactive AI backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants