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

v8 breaking changes mega issue #3539

Open
1 task
pacrob opened this issue Nov 25, 2024 · 5 comments
Open
1 task

v8 breaking changes mega issue #3539

pacrob opened this issue Nov 25, 2024 · 5 comments

Comments

@pacrob
Copy link
Contributor

pacrob commented Nov 25, 2024

What feature should we add?

Collecting breaking issues here.

Tasks

Preview Give feedback
  1. Breaking Change v8
@pacrob pacrob pinned this issue Nov 25, 2024
@pacrob pacrob changed the title web3.py v8 breaking changes mega issue breaking changes mega issue Nov 25, 2024
@pacrob pacrob changed the title breaking changes mega issue v8 breaking changes mega issue Nov 25, 2024
@timolson
Copy link

timolson commented Dec 19, 2024

Sorry if this is the wrong place to comment, but it scares me that 7.x was just recently released and you are already talking about another backwards-incompatible version.

5.x was stable for several years, which is what I'd expect from a major project, so we started our own usage of web3py at the tail end of 5.x. Then 6.x came out and we had to rewrite things. Then 7.x came out and it caused us SO MUCH PAIN. Finding and modifying all our exception handling blocks was rough, especially since test cases often don't cover all the possible breaking code paths for things like network errors. We haven't even released our product yet and we've had to go through two major breaking updates of web3py. IMO it's causing us more problems in maintenance than just using raw eth-rpc in the first place, and TBH I'm regretting choosing web3py at all.

I would please ask you to consider your user base. Web3 involves MONEY and causing breaking changes every 18 months, for just one of the many libraries we rely on, is a major risk to our product and our users. If you are planning to release 8.x and subsequently drop support for 7.x within a 2-year timeframe, then I will probably decide to rip web3py out of our codebase entirely and just rely on raw ethrpc calls which don't change all the damn time.

@kclowes
Copy link
Collaborator

kclowes commented Dec 19, 2024

Thanks for the feedback @timolson . This is just the issue that we use to track what changes we'd like to see in v8, we don't have imminent plans for a v8 release. However, we do need to be able to keep up with the ecosystem, which sometimes means breaking changes. Generally, our breaking changes and our major release cycles coincide with Python removing support for versions - like dropping Python 3.8 listed above. We acknowledge that v7 was a particularly big change and do not anticipate that v8 will be as big of a shift. We had the opportunity to work on many long-standing issues and highly anticipated features for the v7 release (batching, subscribe, etc.), which necessitated some bigger refactors.

I'm curious what was the most painful to upgrade, and if you have any ideas that might help smooth the process in the future. We try to add deprecation warnings, and document changes well in both release notes and the migration guide.

If it helps, we can consider expanding the window of the previous version support. Historically, we've supported the previous version for ~6 months after the stable release. We typically add an issue (see #1575, #3044) and pin it to make sure people know about the previous version sunsetting, but haven't gotten any engagement. Feel free to chat with us in the Python Discord too if you'd prefer.

@timolson
Copy link

timolson commented Dec 19, 2024

Thanks for listening.

We haven't been able to complete a v7 migration for issues you don't even list in the migration guide, and we've decided to just pin to v6 for as long as we can. I'm not sure we'll ever make the move to 7.

One of the issues I didn't mention involves our removal of the addrdict middleware. We don't want it, and we successfully removed that middleware in v6, but removing it in v7 seems to cause fundamental internal issues in web3py where the contract methods are not found. Similar story for hexbytes. As much as you'd like to think hexbytes is 100% compatible with bytes, it simply is not, and it caused us much pain in our serialization system and elsewhere. So we created our own custom middleware to rip out hexbytes and replace it with native bytes. In v7, web3py seems to assume hexbytes everywhere and we got exceptions deep in the web3py stack like complaining about an ETH-DNS address that was a giant integer instead of an address-formatted string.

I didn't mention these things, because I wouldn't expect you to really test that stuff, but structuring addrdict as middleware implies that it is removable. It was removable in v6 but apparently not so in v7. Hexbytes unfortunately seems to be baked-in everywhere and it seems like we are either forced to use hexbytes or drop web3py. We were able to drop it successfully in v6 but v7 breaks without it.

Also, like I said, the exception handling was painful, because you can't simply search-replace ValueError in the codebase. We had to meticulously review every exception handler in our entire codebase. UGH!!! And we're still not sure if we got it right. It's difficult to create breaking test cases for some of these exception conditions, and it's even more difficult to convince ourselves that we're not missing a catch somewhere.

In terms of making future transitions smooth for your userbase, the obvious thing is to simply keep backward compatibility even if you have a new structure also available. Take the middleware changes, for example. Would it have been so hard to continue support for the old function-based middleware factories? I haven't looked deeply, but is there a fundamental reason why web3py couldn't have supported the old function-factory API and simply converted usage of factory methods into a new-style middleware class behind-the-scenes? e.g. something like this at the top of the add/inject method:

if iscallable(function_or_class):
  function_or_class = BackwardCompatibleMiddleware(function_or_class)

Again, maybe there are deeper reasons, but it seems to me like this would be much much easier for the library to support backward compatibility rather than pushing wholesale compatibility changes onto your users' codebases.

As for our problems hexbytes and attrdict, if you are making breaking changes I might suggest you consider finding a way to make these both truly optional. In our use case, we do things like cache results in DB tables and remote memcaches, and we use our data in some JavaScript based servers as well as our Python processes. Obviously JavaScript isn't working with AttrDicts and hexbytes, and if we import data into Python from either a db or memcache or request queue or anywhere else that would want JSON, we are now missing the apparently essential conversion of certain fields to hexbytes and the conversion of the entire JSON datastructures into AttrDicts. It adds way more work for us than the convenience you assume we'd want. But v7 seems to assume an attrdict/hexbytes structure for everything instead of accepting a natural, native, JSON-able datastructure, which we were successfully using with v6. This is currently the biggest blocker for us to move to v7 and we've currently given up and just pinned to v6 until we decide what to do about an upgrade path.

@timolson
Copy link

timolson commented Dec 19, 2024

Please feel free to delete my previous comment after reading, since it's more of a Discord-style discussion.

The point I would like to have endure in this thread is this:

Web3py is not like a normal FOSS project. It is the basis for financial applications, where the most important features are stability and reliability. IMO this project should be extremely reluctant to ever make any breaking changes. There is a very good reason you can still find COBOL mainframes in the far corners of large insurance companies and investment banks. Sexy new features and refactorings may feel good to contributors, but they are very painful, risky, and time-consuming for mission-critical applications to adopt and maintain.

And please don't take my comments in the wrong way. I'm very grateful to all of you who give your time and energy to free open source software. I would just like to highlight the discrepancy between what seems to be a fast-moving and evolving project, and the needs of (at least some) web3 financial applications to have a stable and reliable foundation on which to build. As my previous comment highlights, major changes often cause things to break that you don't even realize will break, things that are not even considered in the migration document.

IMO I'd much rather see a v6.9.99 rather than a 7.x or 8.x.

Thanks for reading!

@kclowes
Copy link
Collaborator

kclowes commented Dec 20, 2024

Thanks for engaging, these are all fair points. I don't think we need to delete any comments, but I may move them to a new issue once we begin work on v8 in earnest to keep this issue focused.

I realize this isn't exactly the point that you wanted to convey, but just to address a few of the upgrade issues: the attrdict middleware should still be removable. I just ran the integration tests and the core tests for the contract interface without it, and tests pass (or fail if accessing the dot notation) appropriately. However, if you have the time, please open up a reproducible issue so we can help debug. Hexbytes I'm less sure about removability. We did make some changes there, but they were changes to make it closer to the underlying bytes implementation (in theory). Again, we'd be happy to dig in if we can get a reproducible issue.

To your second comment, I don't think it's sustainable to keep all current functionality around in web3 forever. Our account/key management libraries do tend to remain stable though. We are getting an audit on those soon, which will probably come with some changes, but then don't anticipate them changing too much after that. We don't have any plans to remove published old versions if that is what works best for you and your use case. We will definitely consider a longer deprecation cycle though, and longer version support.

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

No branches or pull requests

3 participants