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.

Sign up for GitHub

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

[ANCHOR-339] Add the /event endpoint to Callback API.yml #184

Closed
wants to merge 3 commits into from
Closed

[ANCHOR-339] Add the /event endpoint to Callback API.yml #184

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8fa161d
Add the /event endpoint to Callback API.yml
lijamie98 Jul 17, 2023
feb05c2
Changed /event to /events
lijamie98 Jul 17, 2023
1ff4471
Changed /events to /event
lijamie98 Jul 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions openapi/anchor-platform/Callbacks API.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,3 +466,34 @@ paths:
application/json:
schema:
$ref: './schemas.yml#/components/schemas/Error'
/events:
JakeUrban marked this conversation as resolved.
Show resolved Hide resolved
post:
tags:
- Events
- SEP-24
- SEP-31
summary: Receive a event from the anchor platform.
Copy link
Contributor

Choose a reason for hiding this comment

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

[an event]

operationId: postEvent
description: |
Receive a JSON object representing an event.
requestBody:
content:
application/json:
schema:
type: object
properties:
timestamp:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is timestamp not a part of an event?

type: string
description: |
`timestamp` the current Unix timestamp (number of seconds since epoch) at the time the callback is sent.
payload:
$ref: './schemas.yml#/components/schemas/Event'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is now a good time to revisit the schema?

  • Is the id attribute specific to the event or transaction?
  • Why do we have sep as a top level attribute when it is available inside the transaction object?
  • Why do we have a transaction_error type? Can't we merge this with transaction_status_changed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, do we need a timestamp on each event to ensure business can check if they've already processed an event after the one received? Is that possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Event schema can be further improved. I think we have a couple of days to review.

For id, sep, and the transaction status, I am open to redesign if that's also ok with you.

And we should have a created_at in the Event schema.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we redesigning event schema in the separate PR this change LGTM

responses:
'200':
description: The event is successfully processed and ready to receive next event. The response body is empty.
'400':
description: The event is invalid or rejected. The response body contains the error message.
Comment on lines +494 to +495
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we want to define any non-200 response code. All responses should have 200, and all other response codes will be interpreted as an error and retries will be made according to the AP's strategy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The response code is limited 400. If the business server returns anything other than 400, they will be considered the same way as 400.

The retries can be handled in the next fix version.

Copy link
Contributor

Choose a reason for hiding this comment

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

But 400 itself is not a valid status, so I don't think we should define it in the spec.