AIP-84 Standardize PATCH Behavior Across Endpoints in FastAPI

[bugraoz93]

Description

None Values

Currently, the handling of None values in PATCH requests is inconsistent across different parts of the codebase. In some instances, when no mask is provided, None values are filtered out, while in others, such as the Pool model, all values (including None) are validated and persisted if the payload is valid. The objective of this work item is to standardize PATCH request behaviour regarding None values:

1. When a mask is provided, use the values as specified by the mask.
2. When no mask is provided, ensure the payload is validated independently as a fully-formed database entity, allowing for explicit setting of fields to None.

Database Calls

Avoid making database calls solely to check for existence; instead, delegate this responsibility to the database itself during the primary operation.

Use case/motivation

A standardized approach for PATCH endpoints is needed.

Related issues

Relates to: #42370
Follow Up of: #43102

Are you willing to submit a PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[bugraoz93]

@pierrejeambrun created this as a follow-up for PATCH endpoints. FYI

[pierrejeambrun]

Thanks a lot, great description.

[pierrejeambrun]

Up for grab, #good first issue!

[ajitg25]

Hello @pierrejeambrun,

I went through the related links provided in the description, and here’s my understanding:

To standardize the implementation, I need to add a mask parameter to the function. If the parameter is passed, I will use the specified values; otherwise, I will validate the entire payload.

I have updated the create_or_update_pool function accordingly:

@provide_session
def create_or_update_pool(
    name: str,
    slots: int | None,
    description: str | None,
    include_deferred: bool | None,
    session: Session = NEW_SESSION,
    mask: list[str] | None = None,
) -> Pool:
    """Create a pool with given parameters or update it if it already exists."""
    if not name:
        raise ValueError("Pool name must not be empty")
    pool = session.scalar(select(Pool).filter_by(pool=name))
    if pool is None:
        pool = Pool(pool=name, slots=slots, description=description, include_deferred=include_deferred)
        session.add(pool)
    else:
        if mask:
            if 'slots' in mask:
                pool.slots = slots
            if 'description' in mask:
                pool.description = description
            if 'include_deferred' in mask:
                pool.include_deferred = include_deferred
        else:
            pool.slots = slots
            pool.description = description
            pool.include_deferred = include_deferred
    session.commit()
    return pool

Does this align with the requirements?

Additionally, regarding the description mentioning changes in different parts of the codebase, I found several functions with create_or_update. Are there any others that I should look into? Let me know your thoughts on the scope of changes and the specific files that need to be updated.