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

add UniqueSkippedAsDuplicate field to JobRow #142

Closed
wants to merge 1 commit into from

Conversation

chrboe
Copy link

@chrboe chrboe commented Jan 4, 2024

When inserting a job, the insert may be rejected because the configured uniqueness constraints for the job are not met.
With the current API, this happens silently and without a way for the caller to check for this condition.

Since we already record the UniqueSkippedAsDuplicate in the internal dbadapter.JobInsertResult, we just have to propagate this value to the external client API.

Callers can now check the UniqueSkippedAsDuplicate field of JobRow to check for the condition described above.


This is an RFC PR to keep the discussion on #86 going. I am pretty sure this is the "least breaking" API change possible to implement this.

@bgentry @brandur please advise if you like this approach or if you have other suggestions.

When inserting a job, the insert may be rejected because the configured
uniqueness constraints for the job are not met.
With the current API, this happens silently and without a way for the
caller to check for this condition.

Since we already record the UniqueSkippedAsDuplicate in the internal
dbadapter.JobInsertResult, we just have to propagate this value to the
external client API.

Callers can now check the UniqueSkippedAsDuplicate field of JobRow to
check for the condition described above.
@brandur
Copy link
Contributor

brandur commented Jan 6, 2024

Thanks @chrboe!

Hmm, let me chew on this a bit longer. I think this is good, and I realistically I wouldn't expect this to cause any practical problems, but something feels a little off to me about putting a non-job-row property onto the JobRow struct, which is otherwise a close reflection of the DB schema. I'd prefer if we could find a good way of making this available "out of band" in some way like an alternate return value or something of that nature.

@brandur
Copy link
Contributor

brandur commented Mar 28, 2024

@chrboe Going to close this out in favor of the similar, but slightly different approach in #292. Thanks for taking a stab at this!

@brandur brandur closed this Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants