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

fix: Allow protobuf 6.x #13637

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: Allow protobuf 6.x #13637

wants to merge 2 commits into from

Conversation

parthea
Copy link
Contributor

@parthea parthea commented Mar 10, 2025

Apply changes from googleapis/gapic-generator-python#2352 and googleapis/gapic-generator-python#2347 to 4 packages containing protobuf stubs which are not automatically generated using gapic-generator-python

fix: resolve issue where pre-release versions of dependencies are installed

@parthea
Copy link
Contributor Author

parthea commented Mar 10, 2025

presubmit core_deps_from_source failed https://github.com/googleapis/google-cloud-python/actions/runs/13775554603/job/38523909038

~/work/google-cloud-python/google-cloud-python/packages/google-cloud-access-context-manager ~/work/google-cloud-python/google-cloud-python
nox > Error while collecting sessions.
nox > Sessions not found: core_deps_from_source

This was fixed by 346097d

@parthea parthea marked this pull request as ready for review March 10, 2025 22:04
@parthea parthea requested a review from a team as a code owner March 10, 2025 22:04
@parthea parthea added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 10, 2025
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 10, 2025
"""
Run all tests with pre-release versions of dependencies installed
rather than the standard non pre-release versions.
Pre-releases versions can be installed using
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Pre-releases versions can be installed using
Pre-release versions can be installed using

"""

# Install all dependencies
session.install(".")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want a -e here like in the previous test?

Comment on lines 398 to +399
unit_deps_all = UNIT_TEST_STANDARD_DEPENDENCIES + UNIT_TEST_EXTERNAL_DEPENDENCIES
# Install dependencies for the unit test environment
Copy link
Contributor

Choose a reason for hiding this comment

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

Consistency with the other test:

Suggested change
unit_deps_all = UNIT_TEST_STANDARD_DEPENDENCIES + UNIT_TEST_EXTERNAL_DEPENDENCIES
# Install dependencies for the unit test environment
# Install dependencies for the unit test environment
unit_deps_all = UNIT_TEST_STANDARD_DEPENDENCIES + UNIT_TEST_EXTERNAL_DEPENDENCIES

Comment on lines 402 to +407
system_deps_all = (
SYSTEM_TEST_STANDARD_DEPENDENCIES
+ SYSTEM_TEST_EXTERNAL_DEPENDENCIES
+ SYSTEM_TEST_EXTRAS
)
# Install dependencies for the system test environment
Copy link
Contributor

Choose a reason for hiding this comment

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

Consistency with the other test:

Suggested change
system_deps_all = (
SYSTEM_TEST_STANDARD_DEPENDENCIES
+ SYSTEM_TEST_EXTERNAL_DEPENDENCIES
+ SYSTEM_TEST_EXTRAS
)
# Install dependencies for the system test environment
# Install dependencies for the system test environment
system_deps_all = (
SYSTEM_TEST_STANDARD_DEPENDENCIES
+ SYSTEM_TEST_EXTERNAL_DEPENDENCIES
+ SYSTEM_TEST_EXTRAS
)


# Because we test minimum dependency versions on the minimum Python
# version, the first version we test with in the unit tests sessions has a
# constraints file containing all dependencies and extras that should be installed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Consistency with previous

Suggested change
# constraints file containing all dependencies and extras that should be installed.
# constraints file containing all dependencies and extras.

"protobuf_implementation",
["python", "upb"],
)
def core_deps_from_source(session, protobuf_implementation):
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe make it clear it's the local source, as opposed to checking it out from somewhere?

Suggested change
def core_deps_from_source(session, protobuf_implementation):
def core_deps_from_local_source(session, protobuf_implementation):

# Install dependencies specified in `testing/constraints-X.txt`.
session.install(*constraints_deps)

core_dependencies_from_source = [
Copy link
Contributor

Choose a reason for hiding this comment

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

This list is shorter than the corresponding prerel_deps in the previous test. Can we make that one shorter as well? Or does this list include transitive dependencies that are not included in the other? We should explain why they don't correspond.

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.

2 participants