Skip to content

feat: Support concatenation for root keys of the default message and loaded config - #1054

Open
speeddragon wants to merge 1 commit into
edgefrom
feat/hb_opts_plus_config
Open

feat: Support concatenation for root keys of the default message and loaded config#1054
speeddragon wants to merge 1 commit into
edgefrom
feat/hb_opts_plus_config

Conversation

@speeddragon

Copy link
Copy Markdown
Collaborator

Allow adding an item to the already defined default config in the hb_opts module. For example, adding another route or store to the already configured one.

  • Added root-only +key append and key+ prepend behavior in src/core/resolver/hb_opts.erl:677.
  • Binary, atom, and map values are wrapped in lists; lists remain unchanged.
  • +key+ and nested decorated keys are untouched.
  • Decorated root keys are removed from the merged configuration.
  • Integrated the merge into HB_CONFIG startup loading in src/core/http/hb_http_server.erl:39.

@speeddragon

Copy link
Copy Markdown
Collaborator Author

I was thinking about the use case for "stores", which is a structure with more than 1 key, and we accept either a map or a list.

For the on use case, we access it via the map, and we want to add a new entry (e.g., cache-write) to the on hook configuration. For this, we need another pattern.

Suggestion

++ - Concatenate to list.
+ - Add entry on map.

@samcamwilliams

Copy link
Copy Markdown
Collaborator

+ on a message should be irrelevant because the default is to merge not replace anyway?

@speeddragon

Copy link
Copy Markdown
Collaborator Author

+ on a message should be irrelevant because the default is to merge not replace anyway?

The default on edge is to replace.
The default on this branch is to concatenate into a list.

on hook is expecting it to be a map, so if we want to merge with the current default, we need to provide a different behaviour. Checking the previous type (list, map, ...) doesn't work, because <<"stores">> supports both (map and list).

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.

2 participants