Report id of database row inserted by Send.
Hi there. With reference to PR #41, it would be great if goqite could handle a particular use case I have.
I'm building a REST-ish API. Users submit jobs which are long-running (minutes/hours). Upon submission the API immediately responds with a '202 Accepted', and jobs are added to a goqite queue. According to received wisdom, the response sent to the user should include a 'Location' header with a URL where the user can check job progress, e.g. Location /task/123 where 123 is the ID of the job.
However, since the Send method of goqite.Queue does not return anything, I cannot reliably discern the ID of the item the user submitted. Therefore I can't build the /task/123 URL, and the user can't check progress.
I guess I could embed some identifying information into the job body, but this would need to be generated by the client whereas that's clearly a server-side responsibility. I'd also need to parse many large BLOBs in the database to look-up any item, which would be inefficient.
Consequently, I believe it would be useful if Send could return the ID of the row it inserts. This is trivial in SQL with a returning clause, supported in SQLite since version 3.35.0 (2021-03-12).
PR #41 provides this functionality. However, implementing it resulted in a cascade of changes throughout the code-base, which I appreciate isn't ideal. Happy to discuss alternatives! :smile:
Hi @austinjp . Thank you for the clarification! I think it makes sense to add something like this.
I'm a bit unsure about the API design, however. There are three main approaches I see:
- Make
Sendreturn aMessage, like you've proposed in #41. - Make
Sendreturn anID. - Have
Sendaccept a pointer to a message instead, where the ID can be updated if the send succeeds.
I'll have to think about what approach I like the most. The last one is arguably the least idiomatic, but I kinda like it anyway. Hmm. I'll get back to this.
Excellent, and no worries about taking time! I'm using my hacked version for now.
If it's of interest, my preference might also be your third option. When hacking on your code I initially returned an ID, then thought I'd prefer the Message instead in case of subsequent changes to the DB/schema/etc. Perhaps a Returning type might be useful, in addition to Message?
I'm going to close PR #41 because it contains a bug: the 'common' dir is inside the 'internal' dir, meaning the 'common' items are unavailable to code using my PR. I've fixed this in the latest commit to my fork.
@austinjp I'm trying out a change in #43. Will think about it a bit. I'm worried the pointer design would be too magic (non-obvious) for my taste.
I'm worried the pointer design would be too magic (non-obvious) for my taste.
Yeah, I can see that. My thinking about returning an object (rather than an ID) was that it would hide the database implementation details to some degree, and both the object and the database schema could be extended or updated in the future without breaking changes (hopefully).
Okay, the pointer solution is in #44. Which one is the most intuitive to the caller?
Notes:
- The pointer solution doesn't create noise when calling
Send, but the ID update feels a bit too magic - The ID solution fits nicely with passing
ID's around inExtend/Delete, but will often not be used.
I'm worried the pointer design would be too magic (non-obvious) for my taste.
Yeah, I can see that. My thinking about returning an object (rather than an ID) was that it would hide the database implementation details to some degree, and both the object and the database schema could be extended or updated in the future without breaking changes (hopefully).
The ID is already used elsewhere. It's just an opaque string and not something like a UUID anyway, so I think that's fine.
Which one is the most intuitive to the caller?
In all frankness, I may not be qualified to make that call :nerd_face: Coming from Python, I have little experience with pointers so #43 feels intuitive to me. But someone with experience of Go may prefer #44, so I have to plead ignorance :shrug:
Whichever you prefer! As long as I can meet my use-case, I'm happy :innocent:
Your responsiveness is much appreciated, by the way.
Just a quick update: I haven't forgotten about this, I just can't decide. 😅
Just a quick update: I haven't forgotten about this, I just can't decide. 😅
No worries my dude, no rush. I haven't come to any firm conclusions myself :thinking:. I guess, in #43 the id, err := q.Send(...) approach looks like idiomatic Go to me, while the pointer does seem a tad magical :mage_man: as you say. Coming from Python, I'm a believer that explicit is better than implicit so on balance maybe I prefer #43.
Yeah, I like explicit better as well. I'm just a bit sad that in the arguably main send-and-forget approach, there's additional clutter on the client side, because the id won't be used in the common case: _, err := q.Send(…).
Maybe a SendAndGetThatIDBack would work better? :D
Indeed!
Hmm. Public functions could be something like err := q.Send(…) and id, err := q.SendReturningID(…). This is nice because AFAICT this wouldn't be a breaking change to your API.
Internally, code duplication might be minimised by calling some 'private' third function e.g. q.SendOptionallyReturningID(…) which might use structs for func options and return values which would/wouldn't have an ID field as appropriate.
That way:
- the new functionality is added
- no breaking API changes (I think)
- code duplication between
q.Send(…)andq.SendReturningID(…)is minimal - client-side clutter is none/minimal
- internal implementation can use whatever magic/non-magic you prefer
At this point, though, it's becoming about how best to hide clutter, and from whom! :laughing:
@austinjp third iteration in #45. :D
I like it. Looks cleanest to me in terms of client-side clutter, API consistency, impact on code-base, etc.
@austinjp I renamed the method SendAndGetID, but otherwise I've stuck to the implementation you saw last week. Released in https://github.com/maragudk/goqite/releases/tag/v0.2.3 ! Thank you for all the feedback. 😊
@markuswustenberg Fantastic, thanks for all your efforts! :clinking_glasses: