-
Notifications
You must be signed in to change notification settings - Fork 63
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
Add tfx 1.12 support #202
Add tfx 1.12 support #202
Conversation
Thanks for the PR! 🚀 Instructions: Approve using |
/lgtm |
Approval received from @hanneshapke! ✅ PR is approved. Missing merge command to auto-merge PR! |
@casassg Should we tie this change to a new tfxa version? |
Yes, do you want to do a release? Else I can |
Happy to cut the release later today. |
/no-merge (edited by @casassg to avoid automatic merge) |
Merge request received from @hanneshapke! ✅ PR will be auto-merged once Test suite is green! |
There's a timeout for some of pip resolvers again... sigh |
Bumping up min version supported as otherwise it's a shitshow to maintain. Upgraded to tfx~=1.6 |
Unfortunately, we need to either drop support for feast, not add support for tfx 1.12 or drop support for py 3.7. Current resolver error is caused by https://github.com/tensorflow/tensorflow/blob/master/tensorflow/tools/pip_package/setup.py#L100 which avoid protobuf 3.20 in tensorflow 2.11. This conflicts with latest py3.7 feast dependency for protobuf which is >3.20. Hence it makes resolver not find an option as it's non-resolvable. @hanneshapke lmk wdyt on dropping 3.7 |
@casassg Thank you for looking into this. TFX still supports 3.7. It would be a bummer to diverge from the requirements between tfx and tfxa, but understandable. @rcrowe-google do you know when tfx will drop the 3.7 support? |
/lgtm |
Approval received from @hanneshapke! ✅ PR is approved. Missing merge command to auto-merge PR! |
/merge |
Merge request received from @hanneshapke! ✅ PR will be auto-merged once Test suite is green! |
/merge |
Merge request received from @hanneshapke! ✅ PR will be auto-merged once Test suite is green! |
tests are broken on 3.8, still need to fix |
Abandoning this in favour of #203 . Plan is release 0.5.0 with that change and then drop py 3.7 on 0.6 shortly after (while adding support for latest tfx) |
PR just bumps upper limit.