Skip to content
This repository has been archived by the owner on Mar 15, 2024. It is now read-only.

Implement /v1/retrieval-events handler #4

Merged
merged 1 commit into from
Mar 2, 2023
Merged

Implement /v1/retrieval-events handler #4

merged 1 commit into from
Mar 2, 2023

Conversation

kylehuntsman
Copy link
Contributor

Description

Implements the /v1/retrieval-events handler.

Demo

The demo spins up Postgres in Docker, runs the event recorder Docker container, creates the table in Postgres, POSTs to the container, and then views the event in Postgres via psql.

WindowsTerminal_QwplT41wEs.mp4

data/bad-eventName.json Outdated Show resolved Hide resolved
eventrecorder/config.go Outdated Show resolved Hide resolved
Phase *types.Phase `json:"phase"`
PhaseStartTime *time.Time `json:"phaseStartTime"`
EventName *types.EventCode `json:"eventName"`
EventTime *time.Time `json:"eventTime"`
Copy link
Member

Choose a reason for hiding this comment

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

Are these all pointers because they are optional fields? Validation implementation suggests otherwise. Which leaves the question why pointers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I needed them to be pointers because I need to check if their values are nil to gaurd against missing properties.

Copy link
Member

Choose a reason for hiding this comment

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

I am curious why not check for zero values?

eventrecorder/recorder.go Outdated Show resolved Hide resolved
event.EventTime,
event.EventDetails,
}
_, err := r.db.Exec(ctx, query, values...)
Copy link
Member

Choose a reason for hiding this comment

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

we should do batch insertion here

Copy link
Contributor Author

@kylehuntsman kylehuntsman Mar 2, 2023

Choose a reason for hiding this comment

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

I tried reading the docs and couldn't understand how to make that work. I agree though. I ended up here just to get something working.

@masih masih merged commit 2ec6069 into main Mar 2, 2023
@masih masih deleted the feat/go-api branch March 2, 2023 09:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants