Skip to content

Fix protobuf stubtest errors #8758

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

Merged
merged 1 commit into from
Sep 19, 2022
Merged

Conversation

AlexWaygood
Copy link
Member

Fixes #8745.

I'm a little confused, as this PR is basically a revert of #8735. Stubtest appears to have run with the same environment for the two runs, so I'm not sure why the results are different:

We've had the same stubtest errors two nights in a row, however.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@Akuli
Copy link
Collaborator

Akuli commented Sep 17, 2022

The only difference between the runs I see is that the first has --shard-index 0, while the second has --shard-index 1. The --shard-index works like this:

    if len(args.dists) == 0:
        dists = sorted((typeshed_dir / "stubs").iterdir())
    ...
    for i, dist in enumerate(dists):
        if i % args.num_shards != args.shard_index:
            continue

So we have added a project whose name starts with a letter before p, and now protobuf is stubtested along with a different set of projects than before. But the Ran with the following environment: section matches, so I don't know why the stubtest results would depend on what packages were stubtested previously.

@AlexWaygood
Copy link
Member Author

The only difference between the runs I see is that the first has --shard-index 0, while the second has --shard-index 1. The --shard-index works like this:

A difference in how the daily test is being run would also be insufficient to explain the differences we're seeing in CI. We also need to explain the fact that stubtest passed on the PR run both in #8735 and in this PR. While we could speculate that there might be a bug in the script that somehow leads to different packages interfering with each other in the daily test, those concerns don't apply for stubtest in PR runs -- we can be confident that the package is being tested in a fairly isolated environment in a PR run.

@hauntsaninja hauntsaninja merged commit 97c935a into python:master Sep 19, 2022
@hauntsaninja
Copy link
Collaborator

Merging in case that makes the daily test stops failing. Note that we can use skip = true to make stubtest not check protobuf while we figure out what's causing the sometimes failures.

@AlexWaygood AlexWaygood deleted the more-protobuf branch September 19, 2022 06:43
@JelleZijlstra
Copy link
Member

At runtime google.protobuf.reflection.GeneratedProtocolMessageType.__init__ seems to be just type.__init__, so I don't know why we have the method in the stub at all.

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.

Stubtest failed on Fri Sep 16 2022
4 participants