Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
We're currently inheriting the following from the API:
- nid (used for the site name on which we're running a task)
- type (used for the task type)
These aren't great for DX in that they're not overly precise. It would be better to name them respectively:
site_nametargettask_typeLet's leave as-is.
This could be non-trivial; Aegir Services API work may be required.
Comments
Comment #2
gboudrias CreditAttribution: gboudrias at Praxis Labs Coop commentedNot sure if it's possible (or a good idea) to override argument names.
Wouldn't it be better and easier to create a new Services resource entirely? We could simply do away with task_type.
Comment #3
colanFair enough.
That would solve the problem I was worried about: changing the API after a beta release. We could have a new resource ("saas_site" maybe?) with our desired argument names, but still keep the old "task" one around, the one we're using now (but mark it as deprecated). At the next major release, we just remove support for the "task" resource.
Actually, we can't as we're not limited to clone tasks. I plan on also using Disable, Enable, and Delete. Others could be useful too, like Backup. I actually just re-jigged things a bit in the last couple of days to ensure that the "nid" for these now accepts site names (just like we're doing for cloning).
Comment #4
gboudrias CreditAttribution: gboudrias at Praxis Labs Coop commentedI just meant that the new resource could be a sort of shortcut to task->clone that doesn't take any other arguments and always clones the site specified in the settings (as it works in hosting_restapi).
Considering your greater use case, it makes more sense to keep using the same resource as far as I can tell.
It's not too late to change the API (we're not in beta yet), but for the sake of consistency it would be better to change the argument in all the resources. Perhaps changing them to "site" and simply detect whether it's an nid or a
hostnamesite name would be best (the old rule of being flexible in the input when possible).I think the task resource is simply too useful to deprecate as a whole. If anything the other resources' Post functions should be deprecated (create, update and delete), as they require a lot more testing and I'm not sure why anyone would need them when "task" exists. I for one am not using them. We could even granularize the permissions of the "task" resource to mirror the existing permission scheme for the other resources.
In short, after reading your comment:
Comment #5
colanThat's a good idea, but it may have to be more general than that. Isn't it possible to run tasks on platforms and servers as well? We'll probably have to call it "entity" instead of "site". So I could pass in the name of the platform or its node ID to run a migrate task on it, for example. The only issue I could see with this is different bundles with the same name, say if we have a platform or server with the same name as a site. Maybe sites would always be matched first?
We can do something like this when a request comes in:
Let's keep releases in alpha until we finalize this.
I agree with everything else.
Comment #6
gboudrias CreditAttribution: gboudrias at Praxis Labs Coop commentedYou're right about the entity VS site thing. I agree with the matching steps.
Comment #7
colanUpdating the component here as this is an API change.
I won't be working on this any time soon so if anyone else feels like working on it, please do.
Comment #8
colanTurns out there's a hook_services_resources_alter() so we can keep the API code as-is, but modify the resource definition if SaaS is enabled. Instead of nid, let's call the first argument target.
Currently looking into this.
Comment #9
colanUpdating title and description.
Comment #11
colanThe task creation argument "nid" has been changed to "target". We still only support sites, but I added some messaging in the code to explain how other target types can be supported with a link to #5.
Other issues can be opened later to add support for other target types.