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

webhook: integrate bot instance with Bottle #29

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

HumorBaby
Copy link
Contributor

@HumorBaby HumorBaby commented Apr 23, 2019

This PR makes use of WSGI middleware and a Bottle plugin to ensure that the bot instance is accessible wherever the Bottle app is visible (bottle is import'd).

There are two main goals of this PR:

  1. Obviate the need for unsightly globals. See:

    # Because I'm a horrible person
    sopel_instance = None

    • The plugin allows @route decorated callbacks to accept an optional bot parameter that corresponds to the actual bot instance, making the webhook callbacks more like normal Sopel callables. For example:
      diff --git a/sopel_modules/github/webhook.py b/sopel_modules/github/webhook.py
      index 9f4bfd6..8c5a88a 100644
      --- a/sopel_modules/github/webhook.py
      +++ b/sopel_modules/github/webhook.py
      @@ -108,8 +108,8 @@ def get_targets(repo):  
      
      
      @bottle.get("/webhook")
      -def show_hook_info():
      -    return 'Listening for webhook connections!'
      +def show_hook_info(bot):
      +    return 'Hi, I\'m {}! You caught me listening for webhook connections!'.format(bot.config.core.nick)
      Omitting the bot parameter if it is not needed would be perfectly acceptable. Thanks to Bottle plugins, no extra work is necessary to decorate webhook callbacks.
  2. Expose the bot instance through bottle

    • The middleware puts the bot in the server environment, making it easily accessible in many contexts. For example:
      diff --git a/sopel_modules/github/formatting.py b/sopel_modules/github/formatting.py
      index 7d5bf10..8db0396 100644                                                       
      --- a/sopel_modules/github/formatting.py                                            
      +++ b/sopel_modules/github/formatting.py                                            
      @@ -17,12 +17,15 @@ Copyright 2015 Max Gurela                                       
      from __future__ import unicode_literals                                            
                                                                                          
      import re                                                                          
      +import bottle                                                                      
      import requests                                                                    
                                                                                          
      from sopel.formatting import color                                                 
                                                                                          
      current_row = None                                                                 
      current_payload = None                                                             
      +the_bot = None                                                                     
                                                                                          
                                                                                          
      def fmt_url(s, row=None):                                                          
      @@ -372,9 +375,10 @@ def shorten_url(url):                                          
                                                                                          
                                                                                          
      def get_formatted_response(payload, row):                                          
      -    global current_row, current_payload                                            
      +    global current_row, current_payload, the_bot                                   
          current_payload = payload                                                      
          current_row = row                                                              
      +    the_bot = bottle.request.environ.get('bot')                                    
                                                                                          
          messages = []                                                                  
          if payload['event'] == 'push':                                                 
      

    Then, each subsequent call has access to the the_bot global variable. Other options include getting the bot instance from bottle.request.environ... in each function as needed, and making the_bot a local variable and passing it to functions as needed.

Hopefully, this can serve as a starting point for #19 since it now exposes the bot instance (so bot.config or the DB can be accessed for settings) in formatting.py, which in my understanding is the current limitation.

Coming soon

  1. A PR based on this one to completely remove the use of globals in webhook.py and instead use the bot instance provided by @bottle.route. Ended up adding it to this PR.
  2. Maybe imports can be fixed too one day... 😁

@dgw
Copy link
Member

dgw commented May 12, 2019

@HumorBaby Switching to use the in operator (#33) created a merge conflict. No rush, but needs sorting when you have a free moment.

@HumorBaby
Copy link
Contributor Author

@HumorBaby Switching to use the in operator (#33) created a merge conflict. No rush, but needs sorting when you have a free moment.

Done.

@dgw dgw added this to the 0.3.0 milestone Aug 10, 2019
Injecting the bot instance into the WSGI server environment obviates the
need for passing around a bot instance or using `global`s.
Functions that have a `bot` parameter (has to be called `bot`, it is not
a matter of position, but name) will receive the managing Sopel instance
as an argument. This precludes the use of `global sopel_instance`.
@HumorBaby
Copy link
Contributor Author

👍

@dgw
Copy link
Member

dgw commented Dec 7, 2019

More merge conflicts, oh boy! 🙁

@dgw dgw modified the milestones: 0.3.0, 0.4.0 Jan 30, 2020
@dgw dgw modified the milestones: 0.4.0, 0.5.0 Oct 19, 2020
@dgw dgw modified the milestones: 0.5.0, 0.6.0 Jun 4, 2024
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