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

PATCH operation shouldn't be required for proxy resources with no updatable properties #719

Open
TimLovellSmith opened this issue Jul 1, 2024 · 2 comments
Assignees

Comments

@TimLovellSmith
Copy link
Member

Describe the bug

For this typespec

@armResourceOperations(TaskHub)
interface TaskHubs {
  @doc("Get a Task Hub")
  get is ArmResourceRead<TaskHub>;
  @doc("Create or update a Task Hub")
  createOrUpdate is ArmResourceCreateOrReplaceSync<TaskHub>;
  @doc("Delete a Task Hub")
  delete is ArmResourceDeleteSync<TaskHub>;
  @doc("List Task Hubs")
  listByParent is ArmResourceListByParent<TaskHub>;
}

@doc("An Task Hub resource belonging to the namespace.")
@parentResource(Namespace)
model TaskHub is ProxyResource<TaskHubProperties> {
  @doc("Task Hub name")
  @key("taskHubName")
  @pattern("^[a-zA-Z0-9-]{3,64}$")
  @segment("taskHubs")
  @path
  @visibility("read", "create")
  name: string;
}

@doc("The properties of Task Hub")
model TaskHubProperties {
  @visibility("read")
  @doc("The status of the last operation.")
  provisioningState?: ProvisioningState;
}

Apparently, the linter complains that TaskHub needs a PATCH operation, even though it has no writable properties.

"OpenApi specification document is missing 'PATCH' actions in 'namespaces/taskhubs' resource type. Resource type having 'Default' routing type must have GET, PUT, PATCH, DELETE & LIST operations."

To Reproduce
Should be possible to have a minimal repro based on the above or private repo file specification/durabletask/DurableTask.Management/main.tsp

Expected behavior
In this case there's no good reason to support PATCH, since there are no mutable properties. And in fact, other linter rules agree, because if you try to add the Update/Patch support, you will get errors about using an empty payload (assuming you take the logical approach of having an empty payload)

Screenshots
?

Desktop (please complete the following information):
?

Additional context
A productivity pain point for teams onboarding new resource types.

@mikeharder mikeharder self-assigned this Jul 2, 2024
@mikeharder
Copy link
Member

@markcowl: What do you think?

@rkmanda
Copy link
Member

rkmanda commented Jul 2, 2024

Agree that this should only be a warning at best for proxy resources

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 📋 Backlog
Status: No status
Development

No branches or pull requests

3 participants