Design plugin feature by tayoogunbiyi · Pull Request #289 · gleitz/howdoi

tayoogunbiyi · 2020-06-24T15:57:02Z

No description provided.

Co-authored-by: c.decal@campus.unimib.it <c.decal@campus.unimib.it>

…ePlugin

…from BasePlugin to it

csr · 2020-06-24T16:48:47Z

Hi @juped & everyone, Eyitayo, Mwiza, and I have been working on a solution to decompose the howdoi codebase into howdoi.py, BasePlugin (base class for all plugins) and StackOverflowPlugin (a subclass of BasePlugin which is used as the default plugin for the user query if no plugins are specified).

We'd appreciate feedback on:

The general code architecture
Coding style and general improvements
Suggestions on how users will be able to install third-party plugins (we thought about having a system that fetches plugins from your GitHub account). In general though, users will be able to manually add plugin files in the plugins folder
Where the cache needs to be stored. Should it be a global variable that all plugins can access?
Everything else that comes to your mind :)

teymour-aldridge · 2020-06-24T16:58:10Z

+
+from cachelib import FileSystemCache, NullCache
+
+from pyquery import PyQuery as pq


I think this makes it easier to understand this.

Suggested change

from pyquery import PyQuery as pq

from pyquery import PyQuery as query_xml

teymour-aldridge · 2020-06-24T16:59:41Z

+        hyperlinks = element.find('a')
+
+        for hyperlink in hyperlinks:
+            pquery_object = pq(hyperlink)


Again, I think this is a more helpful variable name.

Suggested change

pquery_object = pq(hyperlink)

xml_extractor = query_xml(hyperlink)

teymour-aldridge · 2020-06-24T17:00:36Z

+        for hyperlink in hyperlinks:
+            pquery_object = pq(hyperlink)
+            href = hyperlink.attrib['href']
+            copy = pquery_object.text()


Suggested change

copy = pquery_object.text()

copy = xml_extractor.text()

teymour-aldridge · 2020-06-24T17:00:58Z

+                replacement = copy
+            else:
+                replacement = "[{0}]({1})".format(copy, href)
+            pquery_object.replace_with(replacement)


Suggested change

pquery_object.replace_with(replacement)

xml_extractor.replace_with(replacement)

This feedback is great, we'd like to keep this PR a "refactor" and save these valuable renames for a later PR so that we don't change the previous code too much

teymour-aldridge · 2020-06-24T17:01:31Z

+                replacement = "[{0}]({1})".format(copy, href)
+            pquery_object.replace_with(replacement)
+
+    def get_link_at_pos(self, links, position):


This can be a static method.

teymour-aldridge · 2020-06-24T17:02:05Z

+        except TypeError:
+            return element.text()
+
+    def _get_search_url(self, search_engine):


Suggested change

def _get_search_url(self, search_engine):

@staticmethod

def _get_search_url(search_engine):

teymour-aldridge · 2020-06-24T17:02:51Z

+        return [a.attrib['href'] for a in html('.l')] or \
+            [a.attrib['href'] for a in html('.r')('a')]
+
+    def _extract_links_from_duckduckgo(self, html):


This can also be a static method

Suggested change

def _extract_links_from_duckduckgo(self, html):

@staticmethod

def _extract_links_from_duckduckgo(html):

teymour-aldridge · 2020-06-24T17:03:40Z

+                results.append(parsed_url[0])
+        return results
+
+    def _extract_links(self, html, search_engine):


Suggested change

def _extract_links(self, html, search_engine):

@staticmethod

def _extract_links(html, search_engine):

teymour-aldridge · 2020-06-24T17:04:32Z

+        return filtered_proxies
+
+    def extract(self):
+        print("Hello extract")


I'm not sure why this is here.
If it's intended for it to be overriden in child classes, I'd suggest either raising a NotImplementedError or just leave it as it is (without the print statement).

teymour-aldridge · 2020-06-24T17:05:32Z

+                'Safari/536.5'), )
+
+
+def _random_int(width):


Assuming that this just generates a random number, would it be easier to use random.randint?

teymour-aldridge · 2020-06-24T17:26:48Z

+}
+
+
+class BasePlugin():


Architecture thoughts:

import enum.Enum as Enum class Source(Enum): CLI = 0 DISCORD_MESSAGE = 1 class ServiceConnectionError(Exception): pass class Plugin: name = "UnimplementedPlugin" def __init__(): pass def resolve(self): pass def extract(self, source, data): # raises an exception if this fails pass class PluginStack(list): def __init__(self, plugins): super(PluginStack, self).__init__(plugins) def handle_input(self, source, data): tries = [ ] for plugin in self: try: plugin.extract(source, data) plugin.resolve() except <any of the relevant exceptions>: tries.append("howdoi could not get a response from {}".format(plugin.name))

Further possible areas for improvement:

adding async support.

"real-time" error handling – i.e. if the first plugin fails, howdoi immediately sends a response

optional "supports" attribute for Plugins which is a list containing all the sources they support.

I'm trying to understand PluginStack. Is it the manager of the plugins installed in howdoi?

Yes, that's the idea. Whenver it receives a request it tries each plugin in turn. If a plugin fails, it moves on to the next plugin, otherwise it returns the result of that plugin. The code above isn't 100% complete.

Thank you. I think the user experience is having the user specify the plugin they want to use with a flag along with their query (at least for now, Hajer is working on a model that automatically detects which plugin is the most appropriate). Do we still need a separate plugin stack class? Maybe if we want to retrieve/update the list of installed plugins...

I think that there has to be a list of installed plugins and a way of resolving/calling them somehow. A class to handle this could still be useful, but it would be a bit different to the PluginStack.

teymour-aldridge · 2020-06-25T10:50:00Z

@@ -0,0 +1,3 @@
+{
+    "python.pythonPath": "/Users/cesaredecal/workspace/Environments/howdoi/bin/python"


You probably shouldn't check this in and instead add it to your .gitignore.

Thank you! I forgot

tayoogunbiyi and others added 8 commits June 24, 2020 15:27

Initial refactor of howdoi.py into separate plugin

d16a1cd

Co-authored-by: c.decal@campus.unimib.it <c.decal@campus.unimib.it>

Deleted ununsed functions from howdoi.py, imported get_proxies in Bas…

cf6f386

…ePlugin

Created StackOverflowPlugin and moved StackOverflow specific methods …

c920b9e

…from BasePlugin to it

refactored plugin directory

1574aa2

cleaning up the plugins to be modular

e2b57f2

restore functionality to initial state

383b252

remove call to clear_cache in howdoi.py

e0e3164

ensure to use cache initialized in outer scope

0cafae7

teymour-aldridge reviewed Jun 24, 2020

View reviewed changes

csr added 2 commits June 25, 2020 11:01

Delete duplicate function format_output from StackOverflowPlugin

9121577

Created utils file and refactored StackOverflow to use less code

aaa3013

teymour-aldridge reviewed Jun 25, 2020

View reviewed changes

csr and others added 5 commits June 25, 2020 12:54

Delete settings.json

40aedbf

clean up howdoi file and put constants in standalone file

4506c35

extracted constants to standalone file

619d3ea

extracted unicode handling to constants

cd9fd57

applying linting on utils.py

53d0b71

gleitz force-pushed the master branch 4 times, most recently from 63b79c0 to 0175d81 Compare September 13, 2020 20:09


		from cachelib import FileSystemCache, NullCache

		from pyquery import PyQuery as pq

	from pyquery import PyQuery as pq
	from pyquery import PyQuery as query_xml

	pquery_object = pq(hyperlink)
	xml_extractor = query_xml(hyperlink)

	pquery_object.replace_with(replacement)
	xml_extractor.replace_with(replacement)

	def _get_search_url(self, search_engine):
	@staticmethod
	def _get_search_url(search_engine):

	def _extract_links_from_duckduckgo(self, html):
	@staticmethod
	def _extract_links_from_duckduckgo(html):

	def _extract_links(self, html, search_engine):
	@staticmethod
	def _extract_links(html, search_engine):

		@@ -0,0 +1,3 @@
		{
		"python.pythonPath": "/Users/cesaredecal/workspace/Environments/howdoi/bin/python"

Conversation

tayoogunbiyi commented Jun 24, 2020

Uh oh!

csr commented Jun 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

csr Jun 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

csr Jun 25, 2020 •

edited

Loading