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

Update Automate to use FE2 API #384

Merged

Conversation

JR-Morgan
Copy link
Member

@JR-Morgan JR-Morgan commented Feb 13, 2025

Updated automate SDK to use new FE2 API

There's only a couple changes:
create_new_version_in_project no longer uses a branchName instead modelId.
Because of this also, it will not create a branch for the user if one doesn't already exist.

To cover this usecase, I've added create_new_model_in_project and get_model to create a model with the given inputs.


Additional, under @gjedlicska 's suggestion, I have updated the graphql object models to use snake_casing, and using a pydantic alias config to ensure json serialization using camelCasing.

Discussed questions
  1. should create_new_model_in_project try and find a model with the existing name and return that? right now it will throw SpeckleException

  2. create_new_version_in_project accepts a model_id... but we compare it against the matching_trigger.payload.model_id, and throw if it doesn't match... To me it would make more sense to EITHER
    - just use this payload.model_id (but I'm not sure if it could when we expect it to be null)
    - Not bother checking, and accept the users given modelId as is...
    No we need to check, its to stop circular automations

  3. create_new_version_in_project now returns a Tuple[Model, Version] instead of Tuple[str,str]
    I think this is a good change, but want to double check... Also may make more sense to simply return Version

    Yes, lets return only a version

@JR-Morgan JR-Morgan requested a review from gjedlicska February 13, 2025 12:34
Copy link

linear bot commented Feb 13, 2025

@JR-Morgan JR-Morgan marked this pull request as draft February 13, 2025 12:34
@JR-Morgan JR-Morgan changed the base branch from main to v3-dev February 13, 2025 12:34
@jsdbroughton
Copy link
Contributor

create_new_version_in_project accepts a model_id... but we compare it against the matching_trigger.payload.model_id, and throw if it doesn't match... To me it would make more sense to EITHER

In a future world, when the multi-model or non-model change triggers exist as planned, payload may not be as reliable a source of intent or information.

Should create_new_version() avoid creating a version in the model that triggered the function by default to prevent infinite loops of doom??

@JR-Morgan
Copy link
Member Author

Should create_new_version() avoid creating a version in the model that triggered the function by default to prevent infinite loops of doom??

I think I may have initially misunderstood what the branch trigger checking was doing before. After checking this with Gergo, the check is there for this purpose; to prevent infinite circular loops where one automation triggers itsself.

I chatted with Gergo, and this check remains, and we've exposed different functions for creating/getting models. Hopefully a bit simpler now.

@JR-Morgan JR-Morgan marked this pull request as ready for review February 18, 2025 13:53
@gjedlicska gjedlicska merged commit da6e2d9 into v3-dev Feb 18, 2025
6 checks passed
@gjedlicska gjedlicska deleted the jedd/cxpla-167-update-python-automate-sdk-to-use-fe2-api branch February 18, 2025 14:55
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.

4 participants