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_nametarget
  • task_type Let's leave as-is.

This could be non-trivial; Aegir Services API work may be required.

Comments

colan created an issue. See original summary.

gboudrias’s picture

Not 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.

colan’s picture

Not sure if it's possible (or a good idea) to override argument names.

Fair enough.

Wouldn't it be better and easier to create a new Services resource entirely?

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.

We could simply do away with task_type.

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).

gboudrias’s picture

I 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 hostname site 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:

  • I think focusing our efforts on the task resource is a better idea than deprecating it.
  • For now, simply changing the argument names (and detect the argument type) would suffice.
  • I don't think a new resource fits the greater use case you're describing, although it fits my current use case (but it seems unnecessary now that we've got task working for SaaS).
colan’s picture

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 hostname site name would be best (the old rule of being flexible in the input when possible).

That'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:

  1. Assume the "entity" argument's value is a node ID.
  2. If it's not a number or can't be loaded, search for a matching node title.
  3. If there are multiple node titles, attempt to match a site, then a platform, then a server (in that order).
  4. If there are no matches, return an error.

Let's keep releases in alpha until we finalize this.

I agree with everything else.

gboudrias’s picture

You're right about the entity VS site thing. I agree with the matching steps.

colan’s picture

Component: Aegir SaaS » Aegir Services API

Updating 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.

colan’s picture

Component: Aegir Services API » Aegir SaaS
Assigned: Unassigned » colan

Turns 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.

colan’s picture

Title: Better POST argument names: site_name and task_type » Replace POST argument name "nid" with "target"
Issue summary: View changes

Updating title and description.

  • colan committed ced7bef on 7.x-3.x
    Issue #2724403 by colan: Switch SaaS task creation argument from "nid"...
colan’s picture

Status: Active » Fixed
Issue tags: +API change

The 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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.