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

Remove the option to create a standalone Executor #635

Merged
merged 11 commits into from
Sep 8, 2021

Conversation

johny-b
Copy link
Contributor

@johny-b johny-b commented Sep 7, 2021

NOTE: Recommended way of reading this PR is one commit at a time. Commits are quite separate and it's possible that one of them will be totally wrong while others will make sense.

Major things done:

  • Standalone Executor was replaced with a Golem instance in test_payment_platform tests
  • Standalone Executor was replaced with a Golem instance in a bunch of goth tests
  • Executor now has only one way of instantiation, that requires passing _engine

Additional cleanup:

  • Removed budget from yapapi.Golem.execute_tasks. This is a backward-incompatible change, but this param was ignored either way (was passed to the Executor along the _engine, but Executor ignored budget when _engine was provided). So I think this is fine.
  • Removed depreciated package param from Executor.__init__
  • Executor is no longer an AsyncContextManager. I changed this because there was nothing in __aenter__/__aexit__ that couldn't be removed/replaced without any additional cost.

@johny-b johny-b marked this pull request as ready for review September 7, 2021 15:26
@johny-b johny-b requested review from a team, kmazurek, shadeofblue and filipgolem September 7, 2021 15:26
@shadeofblue
Copy link
Contributor

* Removed depreciated `package` param from `Executor.__init__`

quick nitpick: it's "deprecated", not "depreciated" ...

Copy link
Contributor

@shadeofblue shadeofblue left a comment

Choose a reason for hiding this comment

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

all-ok-here-29195232

@johny-b johny-b merged commit e034735 into master Sep 8, 2021
@johny-b johny-b deleted the johny-b/633-remove-standalone-executor branch September 21, 2021 10:44
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