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

feat: allow catch verbs to receive request types as any #2398

Merged
merged 6 commits into from
Aug 19, 2024
Merged

Conversation

matt2e
Copy link
Collaborator

@matt2e matt2e commented Aug 16, 2024

closes #2213

builtin.CatchRequest now has the following features:

  • the associated request type can be any to allow for catching multiple verbs with different request types
    • The request will be unmarshalled to primitives (like string, int, map, array etc) rather than ftl structs
  • Includes the verb that failed as a builtin.Ref, which is a data type with module and name strings
  • Includes the request type as a string, eg: example.Input, String, etc
    • Originally I had this as a builtin.Ref but that doesnt work for non-ref types

There is certainly room for improvement in terms of how we want to expose these schema definitions, but this PR doesn't try go too far down that road.

@matt2e matt2e requested review from a team and wesbillman and removed request for a team August 16, 2024 03:06
@ftl-robot ftl-robot mentioned this pull request Aug 16, 2024
@matt2e matt2e added the run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue label Aug 16, 2024
Copy link
Collaborator

@alecthomas alecthomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool.

backend/controller/controller.go Outdated Show resolved Hide resolved
backend/controller/pubsub/integration_test.go Outdated Show resolved Hide resolved
@matt2e matt2e added this pull request to the merge queue Aug 19, 2024
Merged via the queue into main with commit dfefe8c Aug 19, 2024
73 checks passed
@matt2e matt2e deleted the matt2e/catch-any branch August 19, 2024 04:19
safeer pushed a commit that referenced this pull request Aug 19, 2024
closes #2213

`builtin.CatchRequest` now has the following features:
- the associated request type can be `any` to allow for catching
multiple verbs with different request types
- The request will be unmarshalled to primitives (like string, int, map,
array etc) rather than ftl structs
- Includes the verb that failed as a `builtin.Ref`, which is a data type
with `module` and `name` strings
- Includes the request type as a string, eg: `example.Input`, `String`,
etc
- Originally I had this as a `builtin.Ref` but that doesnt work for
non-ref types

There is certainly room for improvement in terms of how we want to
expose these schema definitions, but this PR doesn't try go too far down
that road.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-all A PR with this label will run the full set of CI jobs in the PR rather than in the merge queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allowing catch to work with any request types
2 participants