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.

Sign up for GitHub

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

Jump to bottom

Fix push_to_hub by not calling create_branch if PR branch #7069

Merged
merged 6 commits into from
Jul 30, 2024
+6 −5
Merged

Fix push_to_hub by not calling create_branch if PR branch #7069

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
06b076a
Fix push_to_hub by not calling create_branch if branch exists
albertvillanova Jul 25, 2024
857b73c
Fix push_to_hub by not calling create_branch if branch exists
albertvillanova Jul 25, 2024
e4148ae
Reword comment
albertvillanova Jul 25, 2024
dbabdda
Fix push_to_hub by not calling create_branch if PR ref
albertvillanova Jul 30, 2024
ec8b690
Merge branch 'main' into fix-7067
albertvillanova Jul 30, 2024
cf9f2b8
Update test
albertvillanova Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/datasets/arrow_dataset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Copy link
Member Author

Choose a reason for hiding this comment

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

Alternatively:

Suggested change
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/"):

Copy link
Member Author

@albertvillanova albertvillanova Jul 30, 2024 •
edited
Loading

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.

Copy link
Member Author

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.

# We check revision_exists because create_branch raises 400/403 error even if branch exists and exist_ok
api.create_branch(repo_id, branch=revision, token=token, repo_type="dataset", exist_ok=True)

if not data_dir:
Expand Down
3 changes: 2 additions & 1 deletion src/datasets/dataset_dict.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1709,7 +1709,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):
# We check revision_exists because create_branch raises 400/403 error even if branch exists and exist_ok
api.create_branch(repo_id, branch=revision, token=token, repo_type="dataset", exist_ok=True)

if not data_dir:
Expand Down
Loading