balena-io / balena-sdk-python

Balena SDK for Python
Apache License 2.0
67 stars 44 forks source link

User want's to be able to use the service name in the service env var methods #335

Closed thgreasi closed 1 year ago

thgreasi commented 1 year ago

User reported that in v12 they were able to pass the service name as a parameter to the balena.models.environment_variables.device_service_environment_variable.create but now in v13 they have to resolve the service id on their own side which adds friction.

jellyfish-bot commented 1 year ago

[thgreasi] This has attached https://jel.ly.fish/7c49929a-f9bb-4505-8681-f304f895d2d0

thibautd commented 1 year ago

This is a big issue because the call to balena.models.device.service_var.get_all_by_device(device_uuid) doesn't return the service ID. So we don't have a way to know which service ID is associated to which variable.

So we basically can't use the remove() API for example.

thgreasi commented 1 year ago

Hi @thibautd, Have you tried using balena.models.service.get_all_by_application? https://github.com/balena-io/balena-sdk-python/blob/master/DOCUMENTATION.md#function-get_all_by_applicationslug_or_uuid_or_id-options--listservicetype

That should allow you to get all service ids for now until we add support for using service names.

thibautd commented 1 year ago

I know I can get the IDs of services, the problem is that balena.models.device.service_var.get_all_by_device(device_uuid) does NOT return the service ID or service name where the variable is set. Even if I can create a map of service name <-> service ID I cannot know on which service is the variable set.

thgreasi commented 1 year ago

Oh I see your point now. Can you give a try to something like the following?

balena.models.device.service_var.get_all_by_device(
    device_uuid,
    {
        "$expand": {
            "service_install": {
                "$expand": {
                    "installs__service": {
                        "$select": "service_name",
                    },
                },
            },
        },
    },
)

This should include the names of the services for each service variable. If that works for your use case, we should perhaps consider changing the get_all_by_device to include those details by default or at least add an example of how this can be achieved.

thibautd commented 1 year ago

This works but the result output is weird. I mean, it seems that a variable can bet set on multiple services ? Because I get a list of installs_service ?

thgreasi commented 1 year ago

The service_install & installs_service arrays, when expanded from a service variable are always single item tuples. The relations under the hood are foreign keys, so the fact that these are returned as arrays is just because of how our OData API works. Let me point you to the respective model types in open-balena-api: https://github.com/balena-io/open-balena-api/blob/3dfba99e409a442aca77a261ff9e2d721f8f876c/src/balena-model.ts#L451 https://github.com/balena-io/open-balena-api/blob/3dfba99e409a442aca77a261ff9e2d721f8f876c/src/balena-model.ts#L442 https://github.com/balena-io/open-balena-api/blob/3dfba99e409a442aca77a261ff9e2d721f8f876c/src/balena-model.ts#L308

otaviojacobi commented 1 year ago

Hello @thibautd Indeed this was missing when the SDK was migrated to the new format, thanks for raising. This should be fixed in https://github.com/balena-io/balena-sdk-python/pull/338 Which allows the the balena.models.device.service_var.set, balena.models.device.service_var.get and balena.models.device.service_var.remove to work with both the id and the service name in the second parameter. There will be a second PR incoming that will add the same functionality in the balena.models.service.var model (which is the app service vars rather than the specific device service env var as this one) which I think I can finish by today and will come back when it is done.

otaviojacobi commented 1 year ago

Hello @thibautd Balena-sdk version 13.2.0 has been released to PyPI with #338 and #340 and it should allow for both device.service_var.set/get/remove and the general service.var.set/get/remove/get_all_by_service to allow using the service_name when querying. The docs have also been updated:

https://docs.balena.io/reference/sdk/python-sdk/#deviceserviceenvvariable https://docs.balena.io/reference/sdk/python-sdk/#serviceenvvariable

Please let me know if you are able to get your side working with v13 or if you face any issues. I am closing for now but I can re-open if you still find any problem with this, thanks for the comprehension.