feat(collections): add setBatch for explicit key/value pairs - #1785
Closed
CaptainAni187 wants to merge 1 commit into
Closed
CaptainAni187 wants to merge 1 commit into
CaptainAni187 wants to merge 1 commit into
Conversation
set() derives each key from its value, via a key string or a keygen
function. When the key is not part of the data - an array of strings, say -
the only way through today is to attach the key to the value anyway, and
sometimes to delete it again inside the keygen.
setBatch(name, items) takes the pairs directly:
collections.setBatch('my-collection', [
{ key: 'a', value: ['some', 'strings'] },
])
Added as a separate function rather than a two-argument overload of set(),
which currently rejects fewer than three arguments with a specific error.
The batching and upload loop is now shared between the two functions rather
than duplicated; set()'s behaviour, including its log output, is unchanged.
Author
|
Closing this for now to tidy up my open PRs. Happy to reopen if it's still useful. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1613
set()derives every key from its value, either from a key string or a keygen function. As #1613 describes, that breaks down when the key isn't part of the data — the workaround is to attach the key to the value anyway, and sometimes to delete it again inside the keygen:setBatch(name, items)takes the pairs directly:Why a separate function rather than overloading
set()The issue asks for
set(name, items)but then notes overloading "isn't great in docs" and that "asetBatch()function might be a cleaner alternative". I went with the separate function, for a reason beyond docs:set()currently rejects fewer than three arguments explicitly —— and there's a test pinning that behaviour (
should throw if only two args passed). A two-argument overload would have to reverse that guard and weaken the error for genuine mistakes, sinceset(name, values)with a forgotten keygen would then be indistinguishable from a batch call. Keeping the two entry points separate leaves that error intact.Happy to switch to an overload if you'd prefer the shape in the issue title.
Implementation notes
The batching and upload loop is now a shared
uploadValues()helper instead of being duplicated.set()'s behaviour is unchanged, including its log output — the helper takes the collection name as written by the caller for logging, so a lazy state reference still logs the way it did before.setBatchvalidates each item and fails with the same style of structured error the module already uses:ILLEGAL_ARGUMENTSwhenitemsisn't an array or an entry isn't an object, andKEY_ERRORwhen an entry has no stringkey.Values are
JSON.stringify'd exactly as inset(), so a value which is itself an array round-trips as one — that's the case the issue calls out.Tests
Six tests added alongside the existing
settests: setting multiple pairs, setting a value that carries no key of its own, and the four validation paths.packages/collections: 98 passing, up from 92 — the six new tests, no other change.pnpm lint: identical output before and after (0 errors, 14 pre-existing warnings).Changeset included as a minor bump.
Note for anyone building locally:
packages/collectionstests need@openfn/language-commonbuilt first, and that needspnpm build:toolsbefore it, otherwise the build fails on an unbuilt@openfn/adaptor-apis.