Feat/vary set - #1112
Conversation
| hb_store:write( | ||
| loaded_device_store(Opts), | ||
| #{ schema_key(Module) => | ||
| term_to_binary(Schema, [compressed]) }, |
There was a problem hiding this comment.
Definitely want to avoid this if we can.
I get the sense the impulse is that we are trying to avoid hb_cache:write here for performance/infinite recursion reasons? If the latter we do have a raw mode of some kind IIRC.
The easiest solution is likely that we should just enforce and exploit that the loaded-device-store must be a hb_store_volatile, and subsequently we can store whole Erlang terms in its ets. I think this would be acceptable if the performance is truly too painful if we store the normal way because when we read from the hb_cache the hashpath would still return a normal AO-Core message result. This flow would be just like hb_cache:read does for remote stores that return full messages (etc) today.
| UserOpts = hb_maps:without(?TEMP_OPTS, Opts, Opts), | ||
| Key = hb_path:hd(Req, UserOpts), |
There was a problem hiding this comment.
Is the UserOpts extraction here necessary? Also, we should add a small inline comment explaining what we are achieving with this as it wasn't immediately intuitive, at-least to me.
| LoadedMsg; | ||
| failure -> | ||
| failure; | ||
| {error, not_found} -> |
There was a problem hiding this comment.
Seems like it is covering for an issue somewhere else -- maybe in development but fixed by now?
| extract(Device, _Opts) when is_map(Device) -> | ||
| {error, {unsupported_device_type, Device}}; | ||
| extract(Module, Opts) when is_atom(Module) -> | ||
| case hb_opts:get(<<"caching-schema">>, false, Opts) of |
There was a problem hiding this comment.
What is caching-schema? Do we mean cache-device-schemas?
| CacheKey = {?MODULE, extract, Module}, | ||
| case erlang:get(CacheKey) of | ||
| {Version, Schema} -> |
There was a problem hiding this comment.
Shouldn't all of this be in hb_device[_load?]? It is a second layer of caching inside the pdict exactly matching loaded-device-store? If so, we should cache this information in the pdict in the same place -- either exactly or in the analogous spot -- in that module.
| module_version(Module) -> | ||
| try Module:module_info(md5) | ||
| catch _:_ -> code:which(Module) | ||
| end. |
There was a problem hiding this comment.
Do we really want to use md5? All of the device atoms should be prefixed with the base58 of the sha256 of the source that made them. Isn't that the identity we want?
| {error, {abstract_code_unavailable, Module, Error}} | ||
| end. | ||
|
|
||
| module_beam(Module) -> |
There was a problem hiding this comment.
I have no idea what is going on from here down in the file. There isn't a single function comment, let alone an inline. We can't merge AI-comprehended-only blobs of 700 lines into the kernel to be called on every single invocation, and if you do comprehend it please document it such that other readers can follow.
| %% block hash length (43 characters), it is used as an ID. If it is parsable as | ||
| %% an integer, it is used as a block height. If it is not present, the current | ||
| %% block is used. | ||
| -spec block(#{ _ => _ }, #{ _ => _ }, map()) -> term(). |
There was a problem hiding this comment.
Instead of map() is #{} or even _ equivalent in our syntax? Or does it have to be #{ _ => _ }? map() is technically correct, but as much as we can minimize the Erlang map/AO message confusion that would be great. If neither work, can we define an opts() instead?
| Client = hb_opts:get( | ||
| relay_http_client, | ||
| hb_opts:get(http_client, ?DEFAULT_HTTP_CLIENT, Opts), | ||
| Opts | ||
| ), |
2 participants
No description provided.