-
Notifications
You must be signed in to change notification settings - Fork 51
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 getting and setting snap config #203
Conversation
3cfccf4
to
07318b2
Compare
a65659c
to
7d62df9
Compare
7d62df9
to
0a22efc
Compare
Because the shape of this data is essentially arbitrary, and therefore cannot cleanly map to anything on the Landscape Server side, database-schema-wise, I'm going to suggest that we just use There's some precedence for doing it this way. |
Yes, the |
cb0c166
to
2d16db1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
TL;DR
This PR:
SnapManager
) & periodically syncing the config (viaSnapMonitor
)snap-http
packageDeets
This started off #202 (conditionally use snap_http, fall back to local module) but ran into some issues while using both
snap-http
& the locallandscape.client.snap
simultaneously:landscape.client.snap
returns adict
whilesnap_http
returnsSnapdResponse
snap_http
'sSnapException
is missing thejson
method inlandscape.client.snap
'sSnapdHttpException
Applied the git submodule suggestion in #202 and created a symbolic link:
landscape.client.snap_http -> snap-http/snap_http
and deletedlandscape.client.snap
.--
Other noteworthy change: Added a new "schema type"
Nested
that allows nesting of "container types" likeList
,Dict
, &KeyDict
. The values of snap configs have arbitrary types and can be deeply nested. Consider values like:Arbitrary values cannot be generally/succinctly represented with the existing "schema types" esp. very very nested values. The only other option is using a type like
Unknown
that doesn't do any schema validation.