Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented May 23, 2025

What do these changes do?

Updated old types into using new pydantic validators pipelines using Annotated types

  • ♻️ Refactors models_library.basic_type
    • Added deprecation notes and doc
    • Upgraded some constraint string types using Annotated[str, Val1, ...] which validates without affecting the actual python type
    • New SafeQueryStr to use e.g. query parameters of string filters (after PR review with @wvangeit )
  • 📝 Started a collection of docs/llm-prompts
    • to assist on coding conventions or rules
    • Added pydantic-annotated-fields.md: should assist upgrading pydantic classes using the old style.

Related issue/s

How to test

cd packages/models-library
make install-dev
pytest -vv tests/test_basic_types.py

Dev-ops

@pcrespov pcrespov self-assigned this May 23, 2025
@codecov
Copy link

codecov bot commented May 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.94%. Comparing base (aec4c1e) to head (913b627).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7749      +/-   ##
==========================================
+ Coverage   87.31%   88.94%   +1.63%     
==========================================
  Files        1838     1458     -380     
  Lines       71454    60515   -10939     
  Branches     1214      474     -740     
==========================================
- Hits        62392    53828    -8564     
+ Misses       8720     6566    -2154     
+ Partials      342      121     -221     
Flag Coverage Δ
integrationtests 64.38% <ø> (+<0.01%) ⬆️
unittests 88.04% <100.00%> (+1.51%) ⬆️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library 93.07% <100.00%> (-0.04%) ⬇️
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration 69.92% <ø> (ø)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 85.13% <ø> (ø)
agent 96.46% <ø> (ø)
api_server 91.58% <100.00%> (+<0.01%) ⬆️
autoscaling 96.07% <ø> (ø)
catalog 92.29% <ø> (ø)
clusters_keeper 99.25% <ø> (ø)
dask_sidecar 91.67% <ø> (+0.22%) ⬆️
datcore_adapter 98.12% <ø> (ø)
director 76.78% <ø> (ø)
director_v2 91.06% <ø> (+0.01%) ⬆️
dynamic_scheduler 96.76% <ø> (ø)
dynamic_sidecar 90.19% <ø> (ø)
efs_guardian 89.79% <ø> (ø)
invitations 93.28% <ø> (ø)
payments 92.63% <ø> (ø)
resource_usage_tracker 89.13% <ø> (+0.21%) ⬆️
storage 87.49% <ø> (-0.08%) ⬇️
webclient ∅ <ø> (∅)
webserver 85.78% <ø> (+0.08%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aec4c1e...913b627. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pcrespov pcrespov changed the title ♻️ Maintenance: Updates on new Annotated type style and tools ♻️ Maintenance: Updates on new Annotated type style and llm-prompts May 23, 2025
@pcrespov pcrespov added t:maintenance Some planned maintenance work a:models-library labels May 23, 2025
@pcrespov pcrespov added this to the Bazinga! milestone May 23, 2025
@pcrespov pcrespov marked this pull request as ready for review May 23, 2025 18:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors existing custom string types to the new Pydantic v2 Annotated/StringConstraints style (including introducing SafeQueryStr), updates all related usage in the API and tests, and adds guidance in docs/llm-prompts for migrating Pydantic fields to the Annotated pattern.

  • Refactors basic_types to Annotated[str, StringConstraints] and adds _SHORT_TRUNCATED_STR_MAX_LENGTH, _LONG_TRUNCATED_STR_MAX_LENGTH, and SafeQueryStr
  • Updates API dependencies and test suites to use the new types and constants
  • Adds a new docs/llm-prompts/pydantic-annotated-fields.md with migration prompts

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
services/api-server/src/.../models_schemas_solvers_filters.py Switched query parameters from str to SafeQueryStr
packages/models-library/tests/test_projects.py Updated project patch test to use _LONG_TRUNCATED_STR_MAX_LENGTH
packages/models-library/tests/test_basic_types.py Revised ShortTruncatedStr tests to use _SHORT_TRUNCATED_STR_MAX_LENGTH and added more scenarios
packages/models-library/src/models_library/rabbitmq_basic_types.py Replaced RPCMethodName subclass with an Annotated type alias
packages/models-library/src/models_library/projects.py Converted ProjectIDStr to an Annotated type alias
packages/models-library/src/models_library/basic_types.py Migrated constrained string classes to Annotated style and added new constants and SafeQueryStr
docs/llm-prompts/pydantic-annotated-fields.md New prompt document for migrating Pydantic Field() to Annotated
Comments suppressed due to low confidence (4)

packages/models-library/src/models_library/projects.py:45

  • Annotated and TypeAlias are used here but neither is imported in this file; add from typing import Annotated, TypeAlias to avoid a NameError.
ProjectIDStr: TypeAlias = Annotated[str, StringConstraints(pattern=UUID_RE_BASE)]

packages/models-library/tests/test_basic_types.py:80

  • [nitpick] This test imports and relies on a private constant (_SHORT_TRUNCATED_STR_MAX_LENGTH); consider exposing a public constant or API for max length to avoid depending on an internal name.
curtail_length = _SHORT_TRUNCATED_STR_MAX_LENGTH

packages/models-library/tests/test_projects.py:11

  • [nitpick] Test is importing and using a private constant (_LONG_TRUNCATED_STR_MAX_LENGTH); consider providing a public API or constant for this value to keep tests from breaking on internal renames.
from models_library.basic_types import _LONG_TRUNCATED_STR_MAX_LENGTH

docs/llm-prompts/pydantic-annotated-fields.md:3

  • [nitpick] The prompt examples assume the user will import Annotated from typing but don't explicitly instruct it; consider adding "Add from typing import Annotated" to ensure completeness.
Please convert all pydantic model fields that use `Field()` with default values to use the Annotated pattern instead.

@pcrespov pcrespov enabled auto-merge (squash) May 23, 2025 18:47
Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

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

thanks. just a small suggestion

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

Thanks ! I just missed an example where you replaced the deprecated idstring

@sonarqubecloud
Copy link

@pcrespov pcrespov merged commit 8362d80 into ITISFoundation:master May 26, 2025
99 of 100 checks passed
@pcrespov pcrespov deleted the mai/basic-types branch May 26, 2025 14:23
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jun 6, 2025
92 tasks
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:models-library t:maintenance Some planned maintenance work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants