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

enable parametrization of Service instances within a single Golem.run_service() call #514

Merged
merged 12 commits into from
Jul 5, 2021

Conversation

shadeofblue
Copy link
Contributor

enable parametrization of Service instances within a single Golem.run_service() call

  • use of service parametrization in simple-service-poc

closes #372

…n_service()` call

+ use of service parametrization in `simple-service-poc`
@shadeofblue shadeofblue requested a review from azawlocki July 2, 2021 12:55
@shadeofblue shadeofblue self-assigned this Jul 2, 2021
@shadeofblue shadeofblue requested a review from a team July 2, 2021 12:55
@shadeofblue shadeofblue changed the title [lacks tests] enable parametrization of Service instances within a single Golem.run_service() call enable parametrization of Service instances within a single Golem.run_service() call Jul 2, 2021
yapapi/golem.py Outdated
Comment on lines 134 to 144
:param num_instances: optional number of service instances to run. Defaults to a single
instance, unless `instance_params` is given, in which case, the Cluster will be created
with as many instances as there are elements in the `instance_params` iterable.
:param instance_params: optional list of dictionaries of keyword arguments that will be passed
to consecutive, spawned instances. The number of elements in the iterable determines the
number of instances spawned, unless `num_instances` is given, in which case the latter takes
precedence.
In other words, if both `num_instances` and `instance_params` are provided,
the Cluster will be created with the number of instances determined by `num_instances`
and if there are too few elements in the `instance_params` iterable, it will results in
an error.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my pleasure :)

Copy link
Contributor

@azawlocki azawlocki left a comment

Choose a reason for hiding this comment

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

Nice! My only concern is that is conflicts with changes made in b0.6 with #509 (for example, both PR's modify signature of Cluster._run_instance()), perhaps it would be better to merge b0.6 with master before merging this PR?

yapapi/services.py Show resolved Hide resolved
@shadeofblue
Copy link
Contributor Author

Nice! My only concern is that is conflicts with changes made in b0.6 with #509 (for example, both PR's modify signature of Cluster._run_instance()), perhaps it would be better to merge b0.6 with master before merging this PR?

we can do either, and i'd merge this one first for the sake of planning ;)

@shadeofblue shadeofblue merged commit 740a02b into master Jul 5, 2021
@shadeofblue shadeofblue deleted the blue/parametrized-services branch July 5, 2021 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

allow parametrization of Service instances
2 participants