Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Allow application services to have an optional 'url' #1056

Merged
merged 4 commits into from Aug 30, 2016

Conversation

kegsay
Copy link
Member

@kegsay kegsay commented Aug 30, 2016

If 'url' is not specified, they will not be pushed for events or queries. This is useful for bots who simply wish to reserve large chunks of user/alias namespace, and don't care about being pushed for events.

This is done by explicitly setting the url field in the registration file to the empty string. This is done to ensure that this is a conscious decision, and not just a forgotten field. We also log when we encounter ASes which will not be pushed.

If 'url' is not specified, they will not be pushed for events or queries. This
is useful for bots who simply wish to reserve large chunks of user/alias
namespace, and don't care about being pushed for events.
@NegativeMjark
Copy link
Contributor

I was going to say that the PR could be called 'Make the appservice "URL" optional in the appservice config'. But it's not really optional. Maybe call this 'Make "URL" optional in the appservice config in a really awkward way to prevent people omitting it by accident'? :)

@NegativeMjark
Copy link
Contributor

NegativeMjark commented Aug 30, 2016

YAML can represent None using ~ or null FWIW. Which might be a clearer way to explicitly omit the URL?

if as_info["url"] == "":
logger.info(
"(%s) Explicitly empty 'url' provided. This application service " +
"will not receive events or queries.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably start the line " will since it makes it easier to spot the spaces. Also I'd be tempted to omit the '+' operator.

@NegativeMjark
Copy link
Contributor

Looks alright apart from the comments.

Change how we validate the 'url' field as a result.
@NegativeMjark
Copy link
Contributor

LGTM

@kegsay kegsay merged commit 998666b into develop Aug 30, 2016
@richvdh richvdh deleted the kegan/appservice-url-is-optional branch December 1, 2016 14:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants