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

updating grpcio version #14259

Merged
merged 6 commits into from Oct 22, 2023
Merged

updating grpcio version #14259

merged 6 commits into from Oct 22, 2023

Conversation

defreng
Copy link
Contributor

@defreng defreng commented Oct 16, 2023

Description

The PR updates the grpcio dependency for the Python SDK.

I also made a change here to no longer fix the grpcio version to a specific one - which makes it very hard to use pulumi in other projects with other dependencies.

Fixes #14258

Checklist

  • I have run make tidy to update any new dependencies
  • I have run make lint to verify my code passes the lint check
    • I have formatted my code using gofumpt
  • I have added tests that prove my fix is effective or that my feature works
  • I have run make changelog and committed the changelog/pending/<file> documenting my change
  • Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version

@github-actions
Copy link

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project
  • /run-codegen - used to test the Pull Request against downstream codegen
  • /run-docs-gen - used to test the Pull Request against documentation generation

@pulumi-bot
Copy link
Contributor

pulumi-bot commented Oct 16, 2023

Changelog

[uncommitted] (2023-10-22)

Miscellaneous

  • [sdk/python] updates grpcio dependency

Copy link
Member

@Frassle Frassle left a comment

Choose a reason for hiding this comment

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

I also made a change here to no longer fix the grpcio version to a specific one

We have historically seen a lot of issues with grpcio releasing bad versions that don't work with Pulumi, which then trigger a cascade of user issues to us when pip starts pulling it in.

We could give them another chance and try unlocking this dependency, but I'd need to discuss this internally with the team to see how we all feel about this. Even if we do OK the unlock I think these should be ~= dependencies not >=.

If we don't approve the full unlock (i.e. keep it as ==1.59.0) we should probably see if we can get dependabot or similar to keep this dependency up to date. Trying to keep track of it manually hasn't worked.

sdk/python/lib/setup.py Outdated Show resolved Hide resolved
sdk/python/requirements.txt Outdated Show resolved Hide resolved
Co-authored-by: Fraser Waters <frassle@gmail.com>
@github-actions
Copy link

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project
  • /run-codegen - used to test the Pull Request against downstream codegen
  • /run-docs-gen - used to test the Pull Request against documentation generation

@defreng
Copy link
Contributor Author

defreng commented Oct 16, 2023

The ~= selector seems to be a good compromise... I added your code suggestions :-)

And dependabot would definitely make sense!

In the future, should there be another breaking release of grpcio, there would also be these two options:

  • if you are quick enough releasing a new version, a grpcio version could be explicitly blocked in the requirements
  • on the other hand, the good thing is that the grpcio version is under control of the user who uses the SDK. So he could easily on his side install an older version of grpcio - without having to wait for a new pulumi release

@github-actions
Copy link

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project
  • /run-codegen - used to test the Pull Request against downstream codegen
  • /run-docs-gen - used to test the Pull Request against documentation generation

@marns93
Copy link

marns93 commented Oct 18, 2023

Any updates on this? :)
I'm very interested to use Python 3.12 with Pulumi.

@Frassle
Copy link
Member

Frassle commented Oct 18, 2023

No major push back internally to trying to unpin this. If you can just run make changelog to generate changelog entry for this.

@defreng
Copy link
Contributor Author

defreng commented Oct 19, 2023

done :-)

@github-actions
Copy link

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project
  • /run-codegen - used to test the Pull Request against downstream codegen
  • /run-docs-gen - used to test the Pull Request against documentation generation

@Frassle Frassle added this pull request to the merge queue Oct 19, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 20, 2023
@Frassle Frassle added this pull request to the merge queue Oct 22, 2023
@Frassle Frassle removed this pull request from the merge queue due to a manual request Oct 22, 2023
@github-actions
Copy link

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project
  • /run-codegen - used to test the Pull Request against downstream codegen
  • /run-docs-gen - used to test the Pull Request against documentation generation

@Frassle Frassle added this pull request to the merge queue Oct 22, 2023
@github-actions
Copy link

PR is now waiting for a maintainer to take action.

Note for the maintainer: Commands available:

  • /run-acceptance-tests - used to test run the acceptance tests for the project
  • /run-codegen - used to test the Pull Request against downstream codegen
  • /run-docs-gen - used to test the Pull Request against documentation generation

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 22, 2023
@Frassle Frassle added this pull request to the merge queue Oct 22, 2023
Merged via the queue into pulumi:master with commit 85b41ec Oct 22, 2023
8 of 9 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Oct 23, 2023
### Features

- [auto/nodejs] Add support for the path option for config operations
  [#14305](#14305)

- [engine] Converters can return diagnostics from `ConvertState`.
  [#14135](#14135)


### Bug Fixes

- [cli] Tightened the parser for property paths to be less prone to
typos
  [#14257](#14257)

- [engine] Fix handling of explicit providers and --target-dependents.
  [#14238](#14238)

- [engine] Fix automatic diffs comparing against output instead of input
properties.
  [#14256](#14256)

- [sdkgen/dotnet] Fix codegen with nested modules.
  [#14297](#14297)

- [programgen/go] Fix codegen to correctly output pulumi.Array instead
of pulumi.AnyArray
  [#14299](#14299)

- [cli/new] `pulumi new` now allows users to bypass existing project
name checks.
  [#14081](#14081)

- [sdk/nodejs] Nodejs now supports unknown resource IDs.
  [#14137](#14137)

- [sdkgen/python] Fix `_configure` failing due to required args
mismatch.
  [#14281](#14281)


### Miscellaneous

- [cli] Pull in fixes from esc v0.5.6
  [#14284](#14284)

- [protobuf] Add a config as property map field to RunRequest and pass
that to the SDK
  [#14273](#14273)

- [sdk/python] updates grpcio dependency
  [#14259](#14259)
@justinvp justinvp mentioned this pull request Oct 23, 2023
github-merge-queue bot pushed a commit that referenced this pull request Oct 23, 2023
### Features

- [auto/nodejs] Add support for the path option for config operations
  [#14305](#14305)

- [engine] Converters can return diagnostics from `ConvertState`.
  [#14135](#14135)


### Bug Fixes

- [cli] Tightened the parser for property paths to be less prone to
typos
  [#14257](#14257)

- [engine] Fix handling of explicit providers and --target-dependents.
  [#14238](#14238)

- [engine] Fix automatic diffs comparing against output instead of input
properties.
  [#14256](#14256)

- [sdkgen/dotnet] Fix codegen with nested modules.
  [#14297](#14297)

- [programgen/go] Fix codegen to correctly output pulumi.Array instead
of pulumi.AnyArray
  [#14299](#14299)

- [cli/new] `pulumi new` now allows users to bypass existing project
name checks.
  [#14081](#14081)

- [sdk/nodejs] Nodejs now supports unknown resource IDs.
  [#14137](#14137)

- [sdkgen/python] Fix `_configure` failing due to required args
mismatch.
  [#14281](#14281)


### Miscellaneous

- [cli] Pull in fixes from esc v0.5.6
  [#14284](#14284)

- [protobuf] Add a config as property map field to RunRequest and pass
that to the SDK
  [#14273](#14273)

- [sdk/python] updates grpcio dependency
  [#14259](#14259)
justinvp added a commit that referenced this pull request Nov 1, 2023
github-merge-queue bot pushed a commit that referenced this pull request Nov 1, 2023
… providers (#14474)

#14259 upgraded grpcio to 1.59.2, to support Python 3.12. Unfortunately,
grpcio >=1.58.0 has a problem where calls to unimplemented gRPC methods
cause a traceback to be emitted to the server's stderr, which affects
Python dynamic providers, which don't implement `DiffConfig`. The result
is a traceback diagnostic shown for the Pulumi program using the dynamic
provider.

This PR fixes this by reverting the change, downgrading grpcio back to
1.56.2.

The existing Python dynamic provider diagnostic test has been extended
to check to make sure there are no unexpected diagnostics, which fails
before the revert, and passes after.

An upstream issue has been opened:
grpc/grpc#34853

The issue to support Python 3.12 has been re-opened: #14258 (currently
blocked on the upstream issue)

Fixes #14442
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.

Installation is broken on Python 3.12
4 participants