Remove middlewares marked for deletion #2982
Conversation
pacrob
force-pushed
the
test-for-default-middlewares
branch
2 times, most recently
from
aa65be3
to
f5e503f
Compare
reedsa
force-pushed
the
test-for-default-middlewares
branch
2 times, most recently
from
9e309e5
to
c134033
Compare
reedsa
force-pushed
the
test-for-default-middlewares
branch
from
c134033
to
63b53c3
Compare
| (validation_middleware, "validation"), | ||
| (abi_middleware, "abi"), # Delete | ||
| (abi_middleware, "abi"), |
There was a problem hiding this comment.
Couldnt delete this just yet, removal causes validation errors due to invalid formats (string not converted to hex, etc).
Tests failing in this build without abi_middleware: https://app.circleci.com/pipelines/github/ethereum/web3.py/4559/workflows/8c5fe87f-fe5f-4a45-97ee-8391158c502f/jobs/308436
There was a problem hiding this comment.
This can be a separate PR, but since all of the tests are failing on the . We should make an issue though to track this removal.eth-tester inbound validation, I suspect there is a bug with either the inbound validation in eth-tester, or with our eth-tester logic that does some type conversions before we send to eth-tester
Never mind, just saw that the integration tests are failing too.
reedsa
changed the title
reedsa
force-pushed
the
test-for-default-middlewares
branch
from
72fe09b
to
47a6cff
Compare
reedsa
force-pushed
the
test-for-default-middlewares
branch
from
8d109b7
to
e88daaa
Compare
reedsa
force-pushed
the
test-for-default-middlewares
branch
from
6460675
to
46494eb
Compare
| middleware performs the following translations for requests and responses. | ||
|
|
||
| * Numeric request parameters will be converted to their hexadecimal representation | ||
| * Numeric responses will be converted from their hexadecimal representations to |
There was a problem hiding this comment.
Do we have this info somewhere else? Even though we're not using the middleware any more, I think this info is still relevant if we can find a good place for it.
There was a problem hiding this comment.
I could be mistaken but I don't actually see any good detail on formatters in the docs. Since this moved to the formatters we should definitely add a section explaining what they do and how they work, etc. I would say that's beyond the scope of this PR but I can create an issue to track it.
edit: created #2994 to track this
| (validation_middleware, "validation"), | ||
| (abi_middleware, "abi"), # Delete | ||
| (abi_middleware, "abi"), |
There was a problem hiding this comment.
This can be a separate PR, but since all of the tests are failing on the . We should make an issue though to track this removal.eth-tester inbound validation, I suspect there is a bug with either the inbound validation in eth-tester, or with our eth-tester logic that does some type conversions before we send to eth-tester
Never mind, just saw that the integration tests are failing too.
What was wrong?
Needed checks for whether default middlewares have changed
Related to Issue #2972
How was it fixed?
Added test to check default middlewares
Todo:
Cute Animal Picture