Add novem.config + Session, fix the kwargs sprawl (0.6)#232
Merged
Conversation
Connection settings (token, api root, profile) were resolved per object
from a shared **kwargs bag, with no way to configure them once for a
process. Introduce a process-wide ConfigManager, exposed as novem.config,
with explicit setters so a client can configure once and then use naked
constructors, while an explicit per-call argument still takes precedence:
novem.config.set_token("...")
p = novem.Plot("my-plot") # uses the global token
p = novem.Plot("my-plot", token=...) # explicit wins
novem/config.py adds NovemConfig, a resolved-connection dataclass, and
resolve(), which layers the global overrides on top of the existing
file/env precedence in get_current_config (left untouched). NovemAPI and
NovemVisAPI take explicit, typed connection and behaviour keyword
arguments instead of rummaging the kwargs bag.
While here, untangle the connection-kwargs handling: centralise the
profile/config_profile aliasing in resolve() and teach get_current_config
the public profile alias, removing the manual remapping repeated across
the CLI; and drop the dead api_root override in _parse_kwargs, which was
re-applying the raw, un-normalised root after __init__.
Plot, Mail, Grid and Doc accepted their content properties through **kwargs and applied them with a dir()-based setattr scan, so the real argument surface was invisible and a typo silently did nothing. Declare the content properties as explicit named keyword arguments. A shared NovemVisAPI._parse_kwargs applies them through their setters, deferring the ones that must run last and in order (data; content then status; mapping then layout), skips connection and behaviour kwargs, and warns once on any unknown extra. Unknown extras stay non-fatal for backwards compatibility: a stale option warns but never raises. shared and tags remain accepted on every vis, as they were under the old scan. Add a config_manager hook on NovemAPI so a constructor can resolve against a specific ConfigManager, as groundwork for bound sessions.
Add novem.Session, a bound configuration context that overrides any of
profile, token, api root or config path and constructs resources bound
to it without mutating the global novem.config defaults:
prod = novem.Session(profile="production")
staging = novem.Session(profile="staging")
prod.Plot("earnings").data = staging.Plot("earnings").data
novem.config.session() derives a session from the current global
overrides. Differently bound sessions coexist independently.
Contributor
Author
|
There's a greater E2E suite coming to gaia, but this needs to land first. |
The CLI forwarded the entire parsed namespace into the library, via NovemAPI(**args), get_current_config(**args) and NovemGQL(**args). Pull out only the connection options instead, through config_from_args(args) and NovemGQL.from_args(args), and teach get_current_config the public profile alias. Add a CliArgs TypedDict so setup() and the command handlers are statically typed, catching key typos and value-type mistakes with no runtime change; the hyphenated keys (api-url, token-name) force the functional TypedDict form. Typing the namespace surfaced and fixed several latent bugs: flags that are lists were treated as scalars (http_post/put, run_job, events, filter), tree is int or str or None, and a couple of signatures quietly accepted the Optional inputs they already handled. The bootstrap and auth flow and the generic config_from_args extractor stay typed as Mapping[str, Any].
Add a configuration and authentication section to the README covering the precedence chain, the novem.config setters, the per-call token, and Session for working against multiple accounts; fix stale novem.no references to novem.io. Expand the Plot/Mail/Grid/Doc class docstrings to cover live operations, constructor-or-attribute content properties, connection resolution and the deferred-apply ordering. Rewrite CLI.md against the current parser. Several examples had rotted: the chart type is now --type (the old -t selects tags), the -v view concept is gone, sharing requires -C to add and -D to remove while a bare -s lists, and the xdg-open invocation had a typo.
NovemAPI.__init__ swallows the content/behaviour kwargs that subclasses handle, and _parse_kwargs is a no-op terminator for the cooperative chain; in both, the catch-all parameter is never referenced. Pyright / Pylance then flags it as unused, inviting a well-meaning deletion that would silently break the contract that extra keyword arguments are accepted (and, at the vis layer, warned about) rather than raising. Rename it to **_kwargs -- the intentionally-unused convention that Pyright, ruff and flake8 all honour -- and note why in a comment.
Reconcile pyright (which Pylance uses) with mypy across the package, without regressing mypy: - nest the shared/tags __setattr__ isinstance checks in NovemVisAPI and NovemJobAPI so a checker does not widen `value` to Any | NovemShare across branches - table: import pandas/numpy under a TYPE_CHECKING guard (matching vis/plot.py) so the optional-dependency None fallback no longer makes pd./np. access and the pd.DataFrame annotations resolve to Optional; bind `row` before the per-level loop; read the NotRequired "username" config key via .get() - add pyrightconfig.json (point pyright at .venv so pandas/numpy resolve as in Pylance/CI; disable reportIncompatibleVariableOverride to match mypy's leniency on the id/_type subclass overrides) - fix the socketio/mcp optional-import type-ignore codes so neither checker trips whether or not the extra is installed mypy and pyright now both report the package clean.
…ssing A library constructor calling sys.exit(0) on a missing-credentials *error* was wrong on three counts: it killed the host process (bad for notebook/in-process callers), it used the success exit code, and it printed the banner to stdout. Replace it with a dedicated NovemAuthError(NovemException) raised from NovemAPI.__init__. The CLI already routes uncaught exceptions to stderr via its excepthook (and exits non-zero), and the token-setup paths catch NovemException, so the CLI now reports missing credentials on stderr with a non-zero exit. Library callers can catch the error instead of being exited. Bump to 0.6.0: this is a behaviour change to the public contract (the missing-credentials path no longer raises SystemExit), alongside the global-config / Session / explicit-kwargs work on this branch.
bjornars
force-pushed
the
bsn/config-kwargs-sessions
branch
from
0bab857 to
776ba23
Compare
Contributor
|
Great stuff. The harmonization of config still allows overrides of api roots and tokens from cli options? E.G. ENV options and on individual Plots and Classes? Because there is a valid use case on operating as multiple users in the same script my specifying tokens on the classes. |
Contributor
Author
You read the README, of course. |
Contributor
Of course ... that is actually really sweet. A couple of questions I did not see in the readme:
|
Contributor
Author
Not entirely sure, all of this should be explored in tests. |
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
2 participants
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.
Finally clobbers the kwargs white whale and lands the connection-config rework.
Highlights
novem.config— set connection defaults once (set_token/set_api_root/use_profile); naked constructors pick them up, explicit per-call args still win.Session— bound connections (token / api_root / profile) for working against several accounts at once without touching the global defaults.Usage is in the README (config/auth + Session sections).
Behaviour change worth a look: a missing-credentials connection now raises
NovemAuthErrorinstead ofsys.exit(), so library callers can catch it (the CLI still exits non-zero on stderr).Bumps to 0.6 given the size of the additions. mypy + pyright both clean, full suite green.