-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 push_to_hub by not calling create_branch if PR branch #7069
Conversation
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. |
cc @Wauplin maybe it's a EDIT: ah actually the issue is opened at huggingface/huggingface_hub#2419 |
src/datasets/arrow_dataset.py
Outdated
@@ -5620,7 +5620,8 @@ def push_to_hub( | |||
) | |||
repo_id = repo_url.repo_id | |||
|
|||
if revision is not None: | |||
if revision is not None and not api.revision_exists(repo_id, revision, repo_type="dataset", token=token): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively:
if revision is not None and not api.revision_exists(repo_id, revision, repo_type="dataset", token=token): | |
if revision is not None and not revision.startswith("refs/pr/"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this modification would be more efficient:
- The modification above will fix the 400 Bad Request error without the additional network call
revision_exists
. - In the case of a 403 Forbidden error, anyway we will raise an error some code lines below when trying to push to the branch if the user does not have write permission.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the other hand, it is true that the call to revision_exists
is a more general solution.
I think we need to make this fix anyway,
|
Comment by @Wauplin: huggingface/huggingface_hub#2426 (comment)
|
does this mean we should use |
If user wants to push some data to a new PR, they can already pass |
ah yes we do pass create_pr in |
Show benchmarksPyArrow==8.0.0 Show updated benchmarks!Benchmark: benchmark_array_xd.json
Benchmark: benchmark_getitem_100B.json
Benchmark: benchmark_indices_mapping.json
Benchmark: benchmark_iterating.json
Benchmark: benchmark_map_filter.json
Show updated benchmarks!Benchmark: benchmark_array_xd.json
Benchmark: benchmark_getitem_100B.json
Benchmark: benchmark_indices_mapping.json
Benchmark: benchmark_iterating.json
Benchmark: benchmark_map_filter.json
|
* Fix push_to_hub by not calling create_branch if branch exists * Fix push_to_hub by not calling create_branch if branch exists * Reword comment * Fix push_to_hub by not calling create_branch if PR ref * Update test
* Fix push_to_hub by not calling create_branch if branch exists * Fix push_to_hub by not calling create_branch if branch exists * Reword comment * Fix push_to_hub by not calling create_branch if PR ref * Update test
* Fix push_to_hub by not calling create_branch if branch exists * Fix push_to_hub by not calling create_branch if branch exists * Reword comment * Fix push_to_hub by not calling create_branch if PR ref * Update test
Fix push_to_hub by not calling create_branch if PR branch (e.g.
refs/pr/1
).Note that currently create_branch raises a 400 Bad Request error if the user passes a PR branch (e.g.
refs/pr/1
).EDIT:
Fix push_to_hub by not calling create_branch if branch exists.Note that currently create_branch raises a 403 Forbidden error even if all these conditions are met:
Fix #7067.
Related issue: