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

feat: Return error on SDK if Starlark run on any step #634

Merged
merged 10 commits into from
May 23, 2023

Conversation

victorcolombo
Copy link
Contributor

@victorcolombo victorcolombo commented May 23, 2023

Description:

Currently Starlark run remote package, run package and run script blocking calls on the SDK have the following behavior:

  1. If an error happens on the APIC side, we return (*StarlarkRunResult == nil, error != nil)
  2. If an error happens on the Starlark side, we return (*StarlarkRunResult != nil, error == nil), with either StarlarkRunResult.[InterpretationError, ValidationErrors, ExecutionError] != nil
  3. If no errors happen on the Starlark side, we return (*StarlarkRunResult != nil, error == nil) with all StarlarkRunResult.[InterpretationError, ValidationErrors, ExecutionError] == nil

This behavior is not very ergonomic, given that most people are only interested if the Starlark succeeded or not, they should be able to do this with a simple err != nil check, which is the Go idiomatic way of dong it.

If the user wants to investigate further the interpreted instructions, the breakdown of which phase failed, etc, this will still be accessible via the StarlarkRunResult.

Is this change user facing?

YES

References (if applicable):

ava-labs/subnet-evm#649

Copy link
Contributor

@h4ck3rk3y h4ck3rk3y left a comment

Choose a reason for hiding this comment

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

LGTM - please add this to RunRemoteStarlarkScriptBlocking

@victorcolombo victorcolombo changed the title feat: Return error if starlark run fails on SDK feat: Return error on SDK if Starlark run on any step May 23, 2023
@victorcolombo victorcolombo merged commit 8a01cff into main May 23, 2023
@victorcolombo victorcolombo deleted the vcolombo/easier-sdk branch May 23, 2023 12:33
victorcolombo pushed a commit that referenced this pull request May 23, 2023
🤖 I have created a release *beep* *boop*
---


##
[0.76.8](0.76.7...0.76.8)
(2023-05-23)


### Features

* Return error on SDK if Starlark run on any step
([#634](#634))
([8a01cff](8a01cff))


### Bug Fixes

* Make printWarningIfArgumentIsDeprecated unit test deterministic
([#633](#633))
([46bbee5](46bbee5))
* Rollback to previous cluster when cluster set fails
([#631](#631))
([0e212c9](0e212c9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: kurtosisbot <kurtosisbot@users.noreply.github.com>
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