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

Add command line flag to avoid overwriting grpc wheels #318

Merged
merged 8 commits into from
Jan 25, 2022

Conversation

allenporter
Copy link
Contributor

@allenporter allenporter commented Jan 21, 2022

grpcio armv7 wheels are broken again, due to this behavior:

To remedy, add a new flag --skip_exists that will remove the local grpc wheel when it already exists in the index before syncing.

This change refactors existing code in check_available_binary to take advantage of its existing test coverage, but it makes the change bigger than to needs to be.

Assuming this direction is good, the rollout steps are:

  • Release a new wheel
  • Update core to set --skip_exists=grpcio
  • Remove the existing 1.43.0 wheel for armv7
  • Bump something
  • Bump something else again to verify the fix worked.

Add a flag --skip_exists that can let us avoid overiting a grpc wheel
built by home assistant builder with a grpc wheel from pypi.
@pvizeli
Copy link
Member

pvizeli commented Jan 25, 2022

We can remove grpcio from the server to trigger a new build. I personally think that we can merge skip_exists and skip_binary into the same function? Not sure if you ever need one without the other?

@allenporter
Copy link
Contributor Author

We can remove grpcio from the server to trigger a new build. I personally think that we can merge skip_exists and skip_binary into the same function? Not sure if you ever need one without the other?

It seems like there shouldn't be a correctness problem. My motivation for splitting it were to make it easier to understand (we've overloaded what --skip_binary means compared to pip, but maybe this doesn't make it that much worse). The other motivation was to just be conservative, really about leaving aiohttp alone.

I've updated this to take your suggestion and remove --skip_existing, and always use --skip_binary. 👍🏼

@pvizeli pvizeli merged commit 4990dbb into home-assistant:master Jan 25, 2022
@allenporter
Copy link
Contributor Author

We can remove grpcio from the server to trigger a new build. I personally think that we can merge skip_exists and skip_binary into the same function? Not sure if you ever need one without the other?

It seems like there shouldn't be a correctness problem. My motivation for splitting it were to make it easier to understand (we've overloaded what --skip_binary means compared to pip, but maybe this doesn't make it that much worse). The other motivation was to just be conservative, really about leaving aiohttp alone.

I've updated this to take your suggestion and remove --skip_existing, and always use --skip_binary. 👍🏼

I think this introduced a problem where --skip_binary is cleared when the second phase runs, so the wheel was clobbered again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants