Skip to content

Conversation

@gante
Copy link
Contributor

@gante gante commented Jul 23, 2024

What does this PR do?

see title :)

  • Fixed test: tests/models/auto/test_processor_auto.py::ProcessorPushToHubTester::test_push_to_hub_dynamic_processor
    • before:
      Screenshot 2024-07-23 at 09 26 38
    • this PR:
      Screenshot 2024-07-23 at 09 26 59

@gante gante requested review from amyeroberts and ydshieh July 23, 2024 08:28
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

with tempfile.TemporaryDirectory() as tmp_dir:
create_repo(random_repo_id, token=self._token)
# exist_ok=True to avoid unlikely deadlocks
create_repo(random_repo_id, token=self._token, exist_ok=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is odd - given the name coming being generated with uuid, I'm surprised we're ever hitting a clash. Could you share a link to a ci run where this happens?

Copy link
Contributor Author

@gante gante Jul 23, 2024

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

given the name coming being generated with uuid, I'm surprised we're ever hitting a clash

agreed 😅 I didn't dive to find the root cause (perhaps something seed related?), since this fix seems to solve the case and should be harmless 👼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@amyeroberts ⚠️ Important note: on the PRs I linked above, the failure is present at every commit! (which shouldn't happen, because of the random uuid)

Copy link
Contributor

Choose a reason for hiding this comment

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

since this fix seems to solve the case and should be harmless 👼

Unfortunately not completely harmless! We want to make sure we're only creating and deleting the same repo in the test.

@gante
Copy link
Contributor Author

gante commented Jul 23, 2024

@amyeroberts Rebasing fixed the persistent error 🤔 Could be related to some older bug, the root PR was a few weeks old.

Closing the PR

@gante gante closed this Jul 23, 2024
@amyeroberts
Copy link
Contributor

It's due to this commit being included after rebase: #32140

@gante
Copy link
Contributor Author

gante commented Jul 23, 2024

@amyeroberts makes complete sense! Thank you :)

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.

3 participants