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

trs/wip/mint-identifiers #239

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

trs/wip/mint-identifiers #239

wants to merge 1 commit into from

Conversation

tsibley
Copy link
Member

@tsibley tsibley commented Jul 19, 2021

I wrote this endpoint over 2 years ago while integrating identifier
minting and barcode label generation, only to end up refactoring the
bulk of it out and generating labels via a CLI command instead. With a
web interface for minting new identifiers in mind, I kept the code
around as a wip branch. I thought of it again as we're thinking about
identifier API integration with the BBI LIMS, so I updated the code to
account for changes that happened in the interim.

We should consider what limits on "n" (if any?) are appropriate. I'm
not sure what reasonable bound would be. Here's the current summary
stats of batch size for all we've minted so far:

min          1
mode     1,040
max     11,900

Stepping back from n, I think what a limit would really seek to prevent
is minting reqs that run "forever". We can't guarantee this just through
n alone (although very small n might approximate it well enough), so
maybe the more appropriate thing would be an explicit time limit. There
is some existing limit on request/response duration already at the web
server level (both at uWSGI and Apache, I believe), so that may provide
enough of a time limit if the corresponding database session is
terminated when the request is. But I'm not sure that's the case.

This also means that we may not be able to use this API as-is to mint
large batches of identifiers without increasing existing
request/response duration limits, possibly to unreasonable levels. An
async API would obviate that issue, but then require a minting job queue
and polling of job status by the client. (We've talked about this in the
past in related convos.)

(Extracted from #230 after discussion.)

I wrote this endpoint over 2 years ago while integrating identifier
minting and barcode label generation, only to end up refactoring the
bulk of it out and generating labels via a CLI command instead.  With a
web interface for minting new identifiers in mind, I kept the code
around as a wip branch.  I thought of it again as we're thinking about
identifier API integration with the BBI LIMS, so I updated the code to
account for changes that happened in the interim.

We should consider what limits on "n" (if any?) are appropriate.  I'm
not sure what reasonable bound would be.  Here's the current summary
stats of batch size for all we've minted so far:

    min          1
    mode     1,040
    max     11,900

Stepping back from n, I think what a limit would really seek to prevent
is minting reqs that run "forever". We can't guarantee this just through
n alone (although very small n might approximate it well enough), so
maybe the more appropriate thing would be an explicit time limit. There
is some existing limit on request/response duration already at the web
server level (both at uWSGI and Apache, I believe), so that may provide
enough of a time limit if the corresponding database session is
terminated when the request is. But I'm not sure that's the case.

This also means that we may not be able to use this API as-is to mint
large batches of identifiers without increasing existing
request/response duration limits, possibly to unreasonable levels. An
async API would obviate that issue, but then require a minting job queue
and polling of job status by the client. (We've talked about this in the
past in related convos.)
@tsibley tsibley changed the title wip! api: Add endpoint for minting identifiers trs/wip/mint-identifiers Jul 23, 2021
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.

1 participant