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

Service.get_payload as an instance method #680

Closed
johny-b opened this issue Sep 24, 2021 · 6 comments
Closed

Service.get_payload as an instance method #680

johny-b opened this issue Sep 24, 2021 · 6 comments

Comments

@johny-b
Copy link
Contributor

johny-b commented Sep 24, 2021

This would be nice:

Golem.run_service(
    ...,
    instance_params=[{'foo': 'bar'}]
)

class MyService(Service):
    def __init__(self, *, foo):
        self.foo = foo
        
    async def get_payload(self):
        image_hash = 'aaa' if self.foo == 'bar' else 'bbb'
        return await vm.repo(image_hash=image_hash)

I understand this could be solved by providing payload in Golem.run_service, instead of async def get_payload(), but I think it's convenient to have all such logic in a single place.

This is not important. But maybe this is easy? Could be useful.

(I found this doesn't work because I wrote this code & it didn't work, so that's not a 100% abstract thing).

@shadeofblue
Copy link
Contributor

hmm.... there's an inherent problem here

  1. payload determines the demand
  2. the demand is published once per Cluster and is independent from the number of instances that end up being spawned
  3. plus, honestly, if we want to differ the image hash, it doesn't seem like it's still the same service ...

anyway, to sum up, I'd say it's by design that the payload cannot differ on a per-instance basis

@shadeofblue
Copy link
Contributor

shadeofblue commented Sep 27, 2021

you could create a factory that creates classes with an overridden class attribute which determines which image is deployed and use that in the Golem.run_service call ...

@johny-b
Copy link
Contributor Author

johny-b commented Sep 27, 2021

OK, I more-or-less understand.

I have a feeling this somehow corresponds to #560.
If we instantiated the Service-based class immediately in golem.run_service() - and I think this would be the best solution for #560 - this should also work.

@shadeofblue
Copy link
Contributor

OK, I more-or-less understand.

I have a feeling this somehow corresponds to #560.
If we instantiated the Service-based class immediately in golem.run_service() - and I think this would be the best solution for #560 - this should also work.

hmm, I don't really see an overlap between payload definition and the instance's lifecycle ...

@shadeofblue
Copy link
Contributor

and I do believe that, if you really want to differentiate payload based some parameter, you do want to differentiate the Service ... and if you don't want to specify those classes explicitly but rather insist on having them created programatically, such a class factory would be the best solution ...

@johny-b
Copy link
Contributor Author

johny-b commented Nov 18, 2022

I no longer think this is worth considering.

@johny-b johny-b closed this as completed Nov 18, 2022
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

No branches or pull requests

2 participants