Skip to content
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

Create a trigger-specific dynamic configuration layer #1217

Open
christophermaier opened this issue Dec 8, 2016 · 4 comments
Open

Create a trigger-specific dynamic configuration layer #1217

christophermaier opened this issue Dec 8, 2016 · 4 comments

Comments

@christophermaier
Copy link
Collaborator

christophermaier commented Dec 8, 2016

Commands executing in the context of a trigger invocation can currently access user-layer dynamic configuration (user specified in trigger definition or through request headers), but cannot access room-layer dynamic configuration.

It may be useful to create a trigger-specific layer that can be applied instead of the room layer when a command executes from a trigger.

A partial workaround for the time being would be to create a user for the trigger and assign them whatever credentials are needed in a user layer.

UPDATE
Turns out that triggers currently behave as 1-on-1 messages with the bot:

room: %Room{id: id,
name: "direct",
provider: @provider_name,
is_dm: true},
. This means that triggers can currently be configured using the "room/direct" layer (in addition to a user-specific layer), but conflating direct human chats with triggers may not be what we ultimately want or need. Providing a specific trigger configuration layer may allow users to more tightly regulate how their triggers are executed.

@vanstee
Copy link
Member

vanstee commented Dec 14, 2016

Is the proposal to have a single trigger layer for all triggers effectively sharing config?

I'm having trouble thinking of a case where I'd need a separate layer of config that wouldn't already either be in the base layer or in some room layer. A trigger layer would solve the issue but seems like it would be duplicating config that already exists somewhere else.

Another idea: Since we already have the notion of running a trigger as a specific user, would it make sense to also provide an in_room attribute that would run the trigger with that room's dynamic config. This would also make it easy to test configuration of a trigger before creating it and wiring it up to external events.

@kevsmith
Copy link
Member

I think in_room is preferable to introducing another configuration layer. The mental model is easy to grok -- "this trigger runs just as if I submitted it in the ops room". It also avoids duplicating configuration entries just to run a trigger which might grow stale over time.

@christophermaier
Copy link
Collaborator Author

It's not a single layer for all triggers; it'd be like a "special room" that triggers would execute in. You'd provide configuration for your bundles just like you would for other rooms.

Specifying a room would be similar; if there aren't trigger-specific needs, then that may be fine.

There may be weirdness due to the fact that we'd be referring to rooms from one provider in another one; I haven't fully refreshed myself on the current implementation, though.

@vanstee
Copy link
Member

vanstee commented Dec 14, 2016

If we go the in_room route, we may want to specifically call it config_from_room or something to avoid people thinking commands would be output in that room. Although now that I'm thinking about it, that could be nice. Probably a separate issue to discuss though.

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

No branches or pull requests

3 participants