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

Will 0.5 Omnibus issue! #37

Closed
skoczen opened this issue Jun 2, 2014 · 16 comments
Closed

Will 0.5 Omnibus issue! #37

skoczen opened this issue Jun 2, 2014 · 16 comments

Comments

@skoczen
Copy link
Owner

skoczen commented Jun 2, 2014

So there have been a number of thoughts swirling in various issues about easier config, disabling/enabling rooms, disabling/enabling plugins, and how to easily develop, debug, and deploy will. This issue aims to pull all of that together, with a cohesive target.

Big things for this release:

  1. Proper configuration support, as mentioned in Fix configuration #14, Allow disabling built-in plugins #31, Accept invitations to rooms #36, Added Travis build configuration #11, and half the human race. Draft is below.
  2. Better documentation on how to set will up in a dev environment, including best-practices for room config, team env control, etc.
  3. Better documentation on how to set will up in a production environment, including best-practices for test running, updates, and deployment.
  4. Better bootstrapping script, to generate base config files, and set up dev and production-ready config for will.

First draft of config.py:

# Welcome to Will's settings.  You'll need to set a few settings below,
# with some optional ones further down.
# ------------------------------------------------------------------------------------
# Required
# ------------------------------------------------------------------------------------
# The following four settings must be set for will to work.

# This is the list of rooms will should join.
ROOMS = ['Testing, Will Kahuna', 'GreenKahuna']
# Default: ALL_ROOMS

# This is the room will will talk to if the trigger 
# is a webhook and he isn't told a specific room. 
# Defaults to the first of ROOMS.
DEFAULT_ROOM = 'GreenKahuna'

# If will isn't accessible at localhost, you must set this for his keepalive to work.
PUBLIC_URL = "http://my-will.herokuapp.com"  # Note no trailing slash.


# This is the list of plugins will should load. This list can contain:
# 
# Built-in core plugins:
# ----------------------
# All built-in modules:     will/plugins
# Built-in modules:         will/plugins/module_name
# Specific plugins:         will/plugins/module_name/plugin.py
#
# Plugins in your will:
# ----------------------
# All modules:              my_will/plugins
# A specific module:        my_will/plugins/module_name
# Specific plugins:         my_will/plugins/module_name/plugin.py

PLUGINS = [
    "will/plugins",
    "my_will/plugins",
]


# All plugins are enabled by default, unless in this list
PLUGIN_BLACKLIST = [
    # Who would deprive will of cookies??
    # "will/friendly/cookies.py",
]


# Where will should look for templates
TEMPLATE_DIRS = [
    "will/templates",
    "my_will/templates",
]


# ------------------------------------------------------------------------------------
# Optional
# ------------------------------------------------------------------------------------

# User handles who are allowed to perform "admin" actions.  Defaults to everyone.
# ADMIN_HANDLES = [
#     "steven",
#     "levi",
# ]

# Port to listen to (defaults to $PORT, then 80.) Set > 1024 to run without elevated permission.
# HTTPSERVER_PORT = "80"  


# Email address to send from, if your will sends emails.
# WILL_DEFAULT_FROM_EMAIL = "will@example.com"

Note: I (@skoczen) won't have proper cycles to implement this stuff until the week of June 23. However, in that week, I will have enough cycles to hopefully get it done. PRs, ideas, and code welcome in the meantime!

@skoczen
Copy link
Owner Author

skoczen commented Jun 2, 2014

Things that come with this:

  • Bundling more plugins into core will
  • Organizing said plugins so folks can easily enable/disable swathes to meet their needs
  • Potentially, support for an @admin_only decorator. @rbp already did this. Sweet!
  • --dev flag, that watches and autoreloads plugins? Eh? Eh?
  • A proper list of the included plugins
  • Travis config
  • On startup, will checks and output config issues

@skoczen skoczen changed the title Will 0.5 Will 0.5 Omnibus issue! Jun 2, 2014
@rbp
Copy link
Contributor

rbp commented Jun 4, 2014

I have implemented a simple admin_only implementation (actually, as a flag to @respond rather than another decorator, I found it to be simpler). Do you want a pull request, or do you already have code (or other plans) for it? Edit: you can see it at https://github.com/rbp/will/tree/admin_only . I confess it was a quick solution, I didn't think much about it, but made sense at the time.

Regarding --dev and autoreload, let me copy/paste what I wrote on #31, to leave it registered here: "I think it's an excellent idea, and it can be useful even outside a dev scenario. So that could be simply a configuration options. That's not to say that a --dev flag wouldn't also be handy (and it could, for instance, auto-set the auto-reload options to True)."

@skoczen
Copy link
Owner Author

skoczen commented Jun 4, 2014

@rbp my plan for admin_only was to ping you and ask if you wouldn't mind submitting a PR with what you had. :) At first thought, in my mind, it'd be ideal as a @admin_only decorator, independent of respond(), but I can be convinced otherwise. Clearly it doesn't make sense on @route and such (at least not until will can authenticate users via github), but in my mind, they also make sense on @hear. That said, you've used these in practice.

Is there a reason you've only had them on respond?

--dev/autoreload - good point, and thanks for pulling that convo over.

@rbp
Copy link
Contributor

rbp commented Jun 4, 2014

There is a reason: I started to implement it as a decorator, and it seemed to me to make more sense as a kwarg :)

BTW, to make it clear: you seem to think that I only implemented it for respond_to, but I did it for both respond_to and hear. The reason why I ended up implementing it as a kwarg was precisely because it seemed to fit with how the rest of the kwargs for these two were implemented.

Or, to put it another way: the fact that only admins can do foo() is tied to the fact that an admin says something, will hears it (or responds to it) and does foo(). If you decorate foo() with `@routeor@periodic``, a``admin_only`` decorator would not make sense.

@skoczen
Copy link
Owner Author

skoczen commented Jun 4, 2014

Yep, that makes complete and total sense. Even as I was typing out that comment, there was a voice in the back of my head saying "so it's really just hear and respond_to, right?". Your implementation sounds exactly right to me. A PR would be great!

@rbp
Copy link
Contributor

rbp commented Jun 4, 2014

Done: #39 :)

@mikn
Copy link

mikn commented Jun 9, 2014

Hey,
A few questions regarding the road map in general!

Do you have any plans on supporting stacking decorators on the same method?
For instance, right now I write a @hear([..]) decorator to catch both the respond_to and hear scenarios because it just seemed easier than writing two methods doing the same thing. Flagging on the message object which type (say, you write both a @hear and a @respond_to decorator on the same method) of decorator triggering it would also be necessary then.
The second use case for this is if you want the method to match more than one distinct regexp in general to avoid making the regexes very complicated. Of course you must make sure that the named match groups are either optional or match in the different regexes.
From what I've gathered, this would require a non-trivial rewrite of how you track the regexes and trigger the corresponding methods, though.

In general the Plugin API is very nice for Will, I think. But the self.say is a weak spot. I think it should be moved to the message object (and make the message an object) instead of on WillPlugin so you don't have to send in the message as well. I can think of no reason to ever override it, which while nice, is about the only advantage. I may be missing some use cases for it though!

I'm also wondering if there is a reason you're using proper threads and not greenlets for the regexp matching? You already have it as a dependency. You're not using a thread pool and reusing the threads which makes it unnecessarily expensive, and also explains why Will is relatively slow to respond. If you want to actually have proper parallellism you must use multiprocessing in Python because of the global interpeter lock (GIL), but you still need to pool it. :)

The easiest way to let people choose what plugins they want to run would be to dump the standard modules into the plugin folder when you do the plumbing, btw. I wouldn't mind that at all at least! There are a couple "admin" plugins that you may want to keep in the core of course. :)

I've been a bit reluctant to do any pull requests addressing these points because it's not very nice to start ripping at the core of a project the first thing you do. :)

@rbp
Copy link
Contributor

rbp commented Jun 12, 2014

@mikn about your suggestion regarding plugin management: the problem with that is that then you don't get updates when the built-in plugins change. I still prefer using repos for that.

@mikn
Copy link

mikn commented Jun 16, 2014

@rbp The admin plugins can and should of course stay in the core of course! But for the rest of the plugins I think this is a limited problem, partly because of how simple the standard plugins are right now (though that might of course change) and partly because when you build your own bot within a bot framework you have no interest in updating plugins without a code review. Once it is deployed you don't want to alter behaviour of the bot without knowing what changed or why first. From my perspective it would just be positive if at least the core flavour plugins (the ones people may want to disable) were part of the creation script rather than part of the core.

If you want a "good" way to include new flavour plugins and pushing them to the bot users, make sure Will announces them when they are available instead or have a response from Will that can list all available flavour plugins and their regexp matches in the current version. But it's just my five cents as a simple user. :)

@bfhenderson
Copy link
Contributor

Any idea when 0.5 will be released?

@skoczen
Copy link
Owner Author

skoczen commented Jun 18, 2014

Yep - should have put it higher in the original post. I won't have time until the week of June 23 (next week), but I should have time that week, and am hoping to have it out by EOD on the 27th. Cross your fingers. :)

@skoczen
Copy link
Owner Author

skoczen commented Jun 23, 2014

It is now the week of the 23, and I'm now digging in. :)

@bfhenderson
Copy link
Contributor

:) good luck

@skoczen
Copy link
Owner Author

skoczen commented Jun 25, 2014

Alright folks, it's EOD wednesday, and I'm happy to say the primary coding is done! Documentation is another matter - all that's been done there is make a list of what needs documented, but that's for tomorrow.

We've switched to bleeding-edge will 0.5 for our team to iron some of the (certain) kinks out - if anyone is up for giving it a whirl tomorrow, that'd be great. At this point, given the lack of real documentation on what's changed, bug reports are the most useful. If you get super stuck, please toss where you got stuck up here, and bail - don't want to waste anyone's time!

Here's the minimal steps to get it running:

  1. pip install git+https://github.com/greenkahuna/will.git@feature/config-based-setup
  2. Change your WILL_TOKEN to be WILL_V1_TOKEN if you're using it.
  3. Add a config.py file (example below - this is coming to the generator, but isn't there yet)
  4. Run will! He's now quite good at diagnosing config issues, and has a host of new features and robustness. Feedback on sticking points/bugs welcome. Please post any issues as comments on this ticket.

Code isn't close to being ready for CR yet, but if you're curious, it lives over at #42 .

Thanks!
-Steven

(Tagging the people who might be interested: @rbp @bfhenderson @dpoirier @michaeljoseph @pcurry @mikn )

Sample config.py:

# Welcome to Will's settings.
# 

# Config and the environment:
# ---------------------------
# Will can use settings from the environment or this file, and sets reasonable defaults.
# 
# Best practices: set keys and the like in the environment, and anything you'd be ok 
# with other people knowing in this file.
# 
# To specify in the environment, just prefix with WILL_ 
# (i.e. WILL_DEFAULT_ROOM becomes DEFAULT_ROOM).  
# In case of conflict, you will see a warning message, and the value in this file will win.



# ------------------------------------------------------------------------------------
# Required settings
# ------------------------------------------------------------------------------------


# The list of plugin modules will should load. 
# Will recursively loads all plugins contained in each module.


# This list can contain:
# 
# Built-in core plugins:
# ----------------------
# All built-in modules:     will.plugins
# Built-in modules:         will.plugins.module_name
# Specific plugins:         will.plugins.module_name.plugin
#
# Plugins in your will:
# ----------------------
# All modules:              plugins
# A specific module:        plugins.module_name
# Specific plugins:         plugins.module_name.plugin
# 
# Plugins anywhere else on your PYTHONPATH:
# -----------------------------------------
# All modules:              someapp
# A specific module:        someapp.module_name
# Specific plugins:         someapp.module_name.plugin


# By default, the list below includes all the core will plugins and
# all your project's plugins.  

PLUGINS = [
    # Built-ins
    "will.plugins.admin",
    "will.plugins.chat_room",
    "will.plugins.devops",
    "will.plugins.friendly",
    "will.plugins.help",
    "will.plugins.productivity",
    "will.plugins.web",

    # All plugins in your project.
    "plugins",
]

# Don't load any of the plugins in this list.  Same options as above.
PLUGIN_BLACKLIST = [
    # "will.plugins.friendly.cookies",      # But who would deprive will of cookies??
    "will.plugins.productivity.hangout",    # Because it requires a HANGOUT_URL
    "will.plugins.productivity.world_time", # Because it requires a WORLD_WEATHER_ONLINE key
]


# ------------------------------------------------------------------------------------
# Potentially required settings
# ------------------------------------------------------------------------------------

# If will isn't accessible at localhost, you must set this for his keepalive to work.
# Note no trailing slash.
# PUBLIC_URL = "http://my-will.herokuapp.com"

# Port to bind the web server to (defaults to $PORT, then 80.)
# Set > 1024 to run without elevated permission.
HTTPSERVER_PORT = "9000"


# ------------------------------------------------------------------------------------
# Optional settings
# ------------------------------------------------------------------------------------

# The list of rooms will should join.  Default is all rooms.
# ROOMS = ['Testing, Will Kahuna',]


# The room will will talk to if the trigger is a webhook and he isn't told a specific room. 
# Default is the first of ROOMS.
# DEFAULT_ROOM = 'Testing, Will Kahuna'


# Fully-qualified folders to look for templates in, beyond the two that 
# are always included: core will's templates folder, and your project's templates folder.
# 
# TEMPLATE_DIRS = [
#   os.path.abspath("other_folder/templates")
# ]


# User handles who are allowed to perform `admin_only` plugins.  Defaults to everyone.
# ADMINS = [
#     "steven",
#     "levi",
# ]


# Mailgun config, if you'd like will to send emails.
# DEFAULT_FROM_EMAIL="will@example.com"
# Set in your environment:
# export WILL_MAILGUN_API_KEY="key-12398912329381"
# export WILL_MAILGUN_API_URL="example.com"


# Logging level
# LOGLEVEL = "DEBUG"

@skoczen
Copy link
Owner Author

skoczen commented Jun 25, 2014

And just to whet a few appetites:

screen shot 2014-06-25 at 4 49 44 pm

screen shot 2014-06-25 at 4 49 36 pm

@skoczen
Copy link
Owner Author

skoczen commented Jun 28, 2014

And, EOD Friday, 0.5 omnibus is done! CR, done, just released.

Bonus: Will now has proper documentation: http://greenkahuna.github.io/will :)

With the number of changes in this thing, I'm sure there are snags in there - please open issues if you hit them!

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

4 participants