Skip to content

Hub test: avoid repo creation deadlocks#32151

Closed
gante wants to merge 1 commit intohuggingface:mainfrom
gante:fix_create_repo
Closed

Hub test: avoid repo creation deadlocks#32151
gante wants to merge 1 commit intohuggingface:mainfrom
gante:fix_create_repo

Conversation

@gante
Copy link
Copy Markdown
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
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

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

@gante
Copy link
Copy Markdown
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