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

API api to add Topology patches in worker manager #436

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

infherny
Copy link
Contributor

The goal is to be able to dynamically edit topologies

@infherny infherny changed the title API to add Topology patches in worker manager API api to add Topology patches in worker manager Oct 16, 2024
@infherny infherny force-pushed the olivier/api_to_add_topology_patches branch 2 times, most recently from 36b2357 to 947bef2 Compare October 16, 2024 16:14
@infherny infherny self-assigned this Oct 16, 2024
@@ -18,6 +20,7 @@ class BaseObject:
metadata: ObjectMetadata
apiVersion: str
kind: str
spec: dict[str, t.Any]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added spec here to be able to load the spec from the patches.

@infherny infherny requested a review from isra17 October 16, 2024 16:16
@infherny infherny force-pushed the olivier/api_to_add_topology_patches branch 2 times, most recently from 2a302eb to 5022cb6 Compare October 16, 2024 17:06
@infherny infherny force-pushed the olivier/api_to_add_topology_patches branch from 5022cb6 to 0240ded Compare October 16, 2024 17:24
@infherny
Copy link
Contributor Author

Im using nox -rs format to format, and Im still having issue in the CI :/

@infherny
Copy link
Contributor Author

I think the PK should include the kind, wdyt. Since the name is not especially unique across kind

@infherny infherny force-pushed the olivier/api_to_add_topology_patches branch from 0240ded to aae70c8 Compare October 16, 2024 18:01
@infherny
Copy link
Contributor Author

I think the PK should include the kind, wdyt. Since the name is not especially unique across kind

I did this change

src/saturn_engine/models/topology_patches.py Outdated Show resolved Hide resolved
sa.JSON, server_default="{}", nullable=False
)

def __init__(self, topology: BaseObject) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

nit, I would recommend to use a classmethod instead (from_topology)

src/saturn_engine/stores/topologies_store.py Outdated Show resolved Hide resolved
bp = Blueprint("topologies", __name__, url_prefix="/api/topologies")


@bp.route("", methods=("PUT",))
Copy link
Member

Choose a reason for hiding this comment

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

route -> patch? /api/topologies could be used one day to actually create components.

Copy link
Member

Choose a reason for hiding this comment

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

PUT /api/topologies/patch

from saturn_engine.utils.sqlalchemy import upsert


def add(*, session: AnySession, topology: BaseObject) -> TopologyPatch:
Copy link
Member

Choose a reason for hiding this comment

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

add -> patch ? topology -> patch ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does the renaming make sens now?

@infherny infherny force-pushed the olivier/api_to_add_topology_patches branch 2 times, most recently from 991482b to 02bc0eb Compare October 17, 2024 12:30
@infherny infherny force-pushed the olivier/api_to_add_topology_patches branch from 02bc0eb to 2281f75 Compare October 17, 2024 12:39
@infherny infherny requested a review from isra17 October 17, 2024 13:39
@infherny infherny merged commit c496e9c into main Oct 17, 2024
3 checks passed
@infherny infherny deleted the olivier/api_to_add_topology_patches branch October 17, 2024 17:33
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.

2 participants