Skip to content

remove required if default in Service, update spec and add openapi MANUL test - #304

Merged
slashburygin merged 2 commits into
masterfrom
fix_service
Apr 22, 2026
Merged

remove required if default in Service, update spec and add openapi MANUL test#304
slashburygin merged 2 commits into
masterfrom
fix_service

Conversation

@slashburygin

Copy link
Copy Markdown
Contributor

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the Service model and its corresponding OpenAPI and manifest specifications by making the user, before, and after fields optional. The review highlights that removing these fields from the required list in the Service_Get OpenAPI schema may negatively impact API clients, as these fields are guaranteed by defaults in the model. Furthermore, suggestions were made to replace mutable list literals with the list callable for default values in the Python model to prevent potential side effects and maintain consistency.

I am having trouble creating individual review comments. Click here to see my feedback.

docs/openapi/openapi_user.yaml (5524)

medium

The removal of user (line 5524), before (line 5526), and after (line 5527) from the required list of the Service_Get schema appears to be a mistake. Service_Get is used for API responses (GET requests). Since these fields have default values defined in the model, they are guaranteed to be present in the response. Removing them from the required list weakens the API contract for clients, as they will now be treated as optional in generated client code.

It appears these fields should have been removed from the Service_Create schema (line 5742) and Service_Update schema (line 5967) instead, where they are currently still marked as required in this file.

genesis_core/elements/dm/models.py (1237-1238)

medium

Improvement opportunity: use the list callable as the default instead of a mutable list literal [] to avoid potential issues with shared state across instances. This is also more consistent with the use of default=dict elsewhere in this file (e.g., line 155).

        default=list,

genesis_core/elements/dm/models.py (1247-1248)

medium

Improvement opportunity: use the list callable as the default instead of a mutable list literal [] to avoid potential issues with shared state across instances. This is also more consistent with the use of default=dict elsewhere in this file (e.g., line 155).

        default=list,

@slashburygin slashburygin changed the title remove required if default in Service remove required if default in Service, update spec and add openapi manual test Apr 20, 2026
@slashburygin slashburygin changed the title remove required if default in Service, update spec and add openapi manual test remove required if default in Service, update spec and add openapi MANUL test Apr 20, 2026
Comment thread docs/openapi/openapi_boot.yaml Outdated
info:
title: Genesis Core v1 Boot API
version: 0.1.dev205+g5e9177075.d20260327
version: 0.0.3.dev0+g5365cb965.d20260417

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems it will be better to update this line only for release versions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok, changed in all places to latest version at this moment and in the test

Comment thread docs/openapi/openapi_status.yaml Outdated
@slashburygin
slashburygin merged commit 677e458 into master Apr 22, 2026
17 of 19 checks passed
@slashburygin
slashburygin deleted the fix_service branch April 22, 2026 16:58
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.

3 participants