[RFC] Alternate middleware configuration API

[voith]

What was wrong?

see #1039
Currently, the middleware configuration is too flexible.
middlewares can be overriden simply doing

w3 = Web3(Web3.HTTPProvider(NODE_HTTP_ADDRESS,  middlewares=[geth_poa_middleware])

Now this is a problem because some of these middlewares are part of web3's core and overriding them can cause undesired behaviour.

Although, I like the idea of middlewares being very configurable, web3 has done one thing very wrong, i.e it has tied a lot of its functionality to some default middlewares.

some comparison with other projects

To understand the problem better I explored the middleware API of Django and Scrapy.

Django supports Onion Middleware but it can still work without plugging in the default middlewares.
Django allows user to configure them through settings.py and the middlewares look like:

MIDDLEWARE = [
    'django.middleware.security.SecurityMiddleware',
    'django.contrib.sessions.middleware.SessionMiddleware',
    'django.middleware.common.CommonMiddleware',
    'django.middleware.csrf.CsrfViewMiddleware',
]    

Even though a user can remove all middlewares, he/she can visually see what they are doing, which is not the case with web3.py

Scrapy on the other hand has a lot of its default behaviour tied to default middlewares. But its middleware configuration is not very flexible. It has BASE_MIDDLEWARES which are combined with USER_DEFINED_MIDDLEWARES.

BASE_MIDDLEWARES = {
	# 'middleware path': 'priority in the stack'
    'scrapy.spidermiddlewares.httperror.HttpErrorMiddleware': 50,
    'scrapy.spidermiddlewares.offsite.OffsiteMiddleware': 500,
}

USER_DEFINED_MIDDLEWARES = {
	'project.Udm1': 0, # udm --> user defined middleware
	'project.Udm2': 100,
}

The BASE_MIDDLEWARES and USER_DEFINED_MIDDLEWARES are combined together based on their priority.

web3.py provides middleware overriding flexibility of django but ties alot of its default behaviour to middlewares like scrapy.

How can it be fixed?

Option 1

Decouple web3.py's default behaviour from middlewares.
The cons of this approach might be that this might involve a lot of code duplication as code from middlewares would now have to be moved their respective function call. This would violate DRY.

Option 2

Do not allow default middlewares to be overwritten and combine them with user defined middlewares. This can be done in three ways:

1. always add user middlewares at the end: middlewares = default_middlewares + user_defined_middlewares. This is a very naive approach, and make the middleware API too inflexible.
2. Assign index order while configuring middlewares. The vacant position will be occupied by default
   middlewares in order

   user_defined_middlewares = {
   	# 'middleware_path': 'index'
   	'udm1': 0,
   	'udm2': 2,
   }
   web3 = Web3(Provider(), middlewares=user_defined_middleware)
   
   default_middlewares = [
   	'default1',
   	'default2',
   ]
   
   combined_middlewares = [
   	'udm1',
   	'default1',
   	'udm2',
   	'default2'
   ]

   The problem with this approach though is that this needs visulation of the entire stack by its index.
   The good thing about this is that this can easily be implemented with the current API.

   middleware_stack = NamedElementOnion(default_middlewares)
   
   for middleware, index in user_defined_middlewares.items():
   	middleware_stack.inject(middleware, layer=index)

3. Assign priorities instead of indexes like scrapy.

   	user_defined_middlewares = {
   		# 'middleware_path': 'index'
   		'udm1': 0,
   		'udm2': 200,
   	}
   	
   	web3 = Web3(Provider(), middlewares=user_defined_middleware)
   	
   	default_middlewares = {
   		'default1': 700 ,
   		'default2': 500,
   	}
   	combined_middlewares = [
   		'udm1',
   		'udm2',
   		'default2',
   		'default1',
   	]

   This is very similar to 2.2 but here a lot of gaps are left between priorities.
   Option 2.2 can give index error if a middleware is added at an unreachable location.
   However, this also needs visualisation of the stack in order.

I'm in favour of option 2.2 as it needs minimal changes and helps solve the problem with very less breaking changes.

cc @carver @pipermerriam @dylanjw

[carver]

Cool, thanks for writing this up. I agree that the current API is problematic.

I think 2.2 could be okay. Naturally, we'd have to add more docs about our default middleware, the order it's in, and how to interject custom middleware.

[fselmo]

closing in favor of #3131