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

Event process receipt #1366

Merged
merged 1 commit into from Jul 11, 2019
Merged

Conversation

kclowes
Copy link
Collaborator

@kclowes kclowes commented Jun 5, 2019

What was wrong?

There was no way to ignore or discard event log errors if they came up during processing.

Related to Issue #1351. Also fixes #1330

How was it fixed?

Added a new optional error kwarg to processReceipt to handle discarding and ignoring errors.

I'll rebase before merging.

Cute Animal Picture

download

@kclowes kclowes force-pushed the event-process-receipt branch 3 times, most recently from b1328d3 to 76e8f0c Compare June 13, 2019 21:30
@kclowes kclowes mentioned this pull request Jun 14, 2019
Copy link
Member

@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some cosmetic stuff about how we expose the API but 👍

docs/contracts.rst Outdated Show resolved Hide resolved
docs/contracts.rst Outdated Show resolved Hide resolved
docs/contracts.rst Outdated Show resolved Hide resolved
"6fbb0e16a6d89c6804c461f65a1b40bb15816040518082815260200191505060405180910390a17"
"f56d2ef3c5228bf5d88573621e325a4672ab50e033749a601e4f4a5e1dce905d481604051808281"
"5260200191505060405180910390a1505600a165627a7a72305820ff79430a04cf654d7b46edc52"
"9ccaa5d7f77607f54bb58210be0c48455292c810029"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@njgheorghita and @kclowes probably already said this a few times but I think it makes sense to start dog-fooding the ethpm APIs and to package these up as ethpm packages to get these blobs of bytecode out of the tests.

web3/contract.py Outdated Show resolved Hide resolved
@kclowes kclowes force-pushed the event-process-receipt branch 2 times, most recently from 00afc6f to d81403d Compare July 2, 2019 03:47
@kclowes
Copy link
Collaborator Author

kclowes commented Jul 2, 2019

@pipermerriam I just have a couple things that could use your 👀 when you get a chance:

  1. Do you have an opinion on where the EventLogErrorFlags class lives? right now it's in web3/event_log_error_flags.py. I had it in web3/_utils/events.py, but that didn't feel quite right since _utils implies private.
  2. Does the IGNORE branch make sense to you in this line? Is there a better way to add a field to an AttributeDict?

Thanks!

docs/contracts.rst Outdated Show resolved Hide resolved
- Add documentation for event log error flags
- Change ValueError to custom error,
- Split tests to handle different MismatchedABI cases, and split error flag tests out
- Make the warn behavior the fallback
@kclowes kclowes merged commit 4ba0173 into ethereum:master Jul 11, 2019
@kclowes kclowes deleted the event-process-receipt branch July 11, 2019 19:53
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.

Warnings during doc build: missing event-log-object
2 participants