Skip to content

Add ConfigStore support#34

Merged
posborne merged 4 commits into
mainfrom
posborne/config-store
Feb 17, 2026
Merged

Add ConfigStore support#34
posborne merged 4 commits into
mainfrom
posborne/config-store

Conversation

@posborne
Copy link
Copy Markdown
Member

Mirroring the JS SDK, we provide a simple get() interface. I opted to not trying to provide a dict facade or similar. That is possible but it doesn't seem to represent a significant ergonomic improvement and might make it more likely that user's fail to keep in mind the potential exception behavior.

As with the Rust SDK, we intelligently use hint information from the host to get values with a big enough buffer if our original size is insufficient.


Potential discussion points:

  • I chose not to make the store look like a dict; this is possible but I don't feel it adds much and might be too lossy a mapping vs. just providing get/contains.
  • We don't have a strongly established resource pattern; here I take the approach of:
    • Context Manager + close
    • We do exit explicitly on the underlying resource rather than relying on the gc (if context manager or close() are used).
    • We don't try to provide an exception if use-after-close is performed, we document that the code will trap.

I expect to rework this code when the unified exception mapping is present, but feel there is enough here to review the important bits.

Copy link
Copy Markdown
Member

@erikrose erikrose left a comment

Choose a reason for hiding this comment

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

As much a note to myself as to you, but…

It looks like what this class adds (in addition to some very nice docs) is…

  1. Auto buffer sizing (yay)
  2. A close() method

Otherwise, the API looks parallel to what we'd get from the generated class, which would be…

from wit_world.imports.config_store import Store

with Store.open("someConfig") as config:
    print(config.get("someKey"), 1024)  # and hope for the best bufferwise

See you in a few minutes! Oh, also, I'd prefer to subsume open() into __init__() so you can just ConfigStore("someStore").

@dgohman-fastly
Copy link
Copy Markdown
Contributor

As with the Rust SDK, we intelligently use hint information from the host to get values with a big enough buffer if our original size is insufficient.

The buffer len arguments are there for the benefit of the component adapter, which has to work with fixed-sized buffers passed in by the user code through the witx ABI. For code using WIT bindings directly, it's ok to just let the host call back into the guest with cabi_realloc with whatever length is needed. So the Python SDK here should be able to just pass the maximum u64 value to all the max-len arguments. That way it'll just never see buffer-len errors.

Mirroring the JS SDK, we provide a simple get() interface.  I opted
to not trying to provide a dict facade or similar. That is possible
but it doesn't seem to represent a significant ergonomic improvement
and might make it more likely that user's fail to keep in mind the
potential exception behavior.

As with the Rust SDK, we intelligently use hint information from
the host to get values with a big enough buffer if our original
size is insufficient.
@posborne posborne force-pushed the posborne/config-store branch from 5bb9633 to e188cba Compare February 4, 2026 03:30
Also, use larger buffer size rather than doing
unecessary resizes and address a few other comments
from the PR.
@posborne posborne force-pushed the posborne/config-store branch from e188cba to e803f69 Compare February 4, 2026 03:34
@posborne
Copy link
Copy Markdown
Member Author

posborne commented Feb 4, 2026

I rebased these changes on the latest upstream; comments that are related can be found on the logging PR related to introduction of an inheritance hierarchy and open() vs __init__ and potential fallibility.

The exceptions that are generated now are buried fairly deep which could be a bit challenging for users to work with. We max need to fix some stuff later on but I tried to document each of those potential exceptions explicitly so end-users know where they may need to dig to import appropriate exceptions.

Comment thread fastly_compute/config_store.py Outdated
Comment thread fastly_compute/config_store.py Outdated
@erikrose
Copy link
Copy Markdown
Member

erikrose commented Feb 5, 2026

The exceptions that are generated now are buried fairly deep which could be a bit challenging for users to work with.

I wrote up a proposal on that: #41.

@posborne posborne requested a review from erikrose February 17, 2026 18:14
Copy link
Copy Markdown
Member

@erikrose erikrose left a comment

Choose a reason for hiding this comment

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

Looks good! Let's get it in.

@posborne posborne merged commit c4b16ee into main Feb 17, 2026
3 checks passed
@posborne posborne deleted the posborne/config-store branch February 17, 2026 21:13
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.

3 participants