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

rm sync_compatible from automations SDK #16584

Merged
merged 14 commits into from
Jan 3, 2025
Merged

rm sync_compatible from automations SDK #16584

merged 14 commits into from
Jan 3, 2025

Conversation

zzstoatzz
Copy link
Collaborator

@zzstoatzz zzstoatzz commented Jan 3, 2025

related to #15008

Copy link

codspeed-hq bot commented Jan 3, 2025

CodSpeed Performance Report

Merging #16584 will not alter performance

Comparing rm-sync-compat-2 (b44cc8b) with main (85f6e26)

Summary

✅ 3 untouched benchmarks

@zzstoatzz zzstoatzz changed the title rm sync compat 2 rm sync_compatible from automations SDK Jan 3, 2025
@github-actions github-actions bot added the enhancement An improvement of an existing feature label Jan 3, 2025
@desertaxle desertaxle removed docs ui-replatform Related to the React UI rewrite labels Jan 3, 2025
@desertaxle
Copy link
Member

Looks like there are some changes from #16583 also in this PR, so I'm going to wait to review until #16583 is merged.

@zzstoatzz
Copy link
Collaborator Author

@desertaxle #16583 is merged

assert name is not None
automation = await client.read_automations_by_name(name=name)
if len(automation) > 0:
return cls(**automation[0].model_dump()) if automation else None
Copy link
Member

Choose a reason for hiding this comment

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

Seems like we should be consistent and always raise if the automation is not found. That should help simplify the typing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good call, applied in bf22753

@zzstoatzz zzstoatzz merged commit 5fb0a5a into main Jan 3, 2025
38 checks passed
@zzstoatzz zzstoatzz deleted the rm-sync-compat-2 branch January 3, 2025 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants