-
Notifications
You must be signed in to change notification settings - Fork 675
fix: file save start_branch as a body attribute #3329
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3329 +/- ##
=======================================
Coverage 95.75% 95.75%
=======================================
Files 98 98
Lines 6051 6053 +2
=======================================
+ Hits 5794 5796 +2
Misses 257 257
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Passing `start_branch` as kwargs results in it being passed as query
argument to the API:
```
send: b'PUT /api/v4/projects/12345678/repository/files/readme.txt?start_branch=main
send: b'{"file_path": "readme.txt", "branch": "new_branch", "content":
"Modified contents", "commit_message": "File was modified for this new
branch"}'
```
which results in error being returned:
```
{"message":"You can only create or edit files when you are on a branch"}
```
It should instead be sent a body attribute, which succeeds in creating
the branch during the save.
To be sent as body attribute it must be specified as concrete function
argument and class attribute instead of just using kwargs
Closes: python-gitlab#3318
6d86c8b to
ad2802a
Compare
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.
Pull request overview
This PR fixes a bug where the start_branch parameter in ProjectFile.save() was being incorrectly passed as a query parameter instead of a body attribute, causing API errors when attempting to create a new branch during file save operations.
Changes:
- Added
start_branchas a class attribute onProjectFilewith default valueNone - Added
start_branchas an explicit parameter to thesave()method signature - Updated the
save()method to assignstart_branchto the instance before calling the parentsave()method
Comments suppressed due to low confidence (1)
gitlab/v4/objects/files.py:24
- The class 'ProjectFile' does not override 'eq', but adds the new attribute branch.
The class 'ProjectFile' does not override 'eq', but adds the new attribute commit_message.
The class 'ProjectFile' does not override 'eq', but adds the new attribute start_branch.
The class 'ProjectFile' does not override 'eq', but adds the new attribute file_path.
class ProjectFile(SaveMixin, ObjectDeleteMixin, RESTObject):
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self, | ||
| branch: str, | ||
| commit_message: str, | ||
| start_branch: str | None = None, |
Copilot
AI
Jan 20, 2026
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.
The start_branch parameter should be included in the functional test coverage. The existing functional test in tests/functional/api/test_repository.py already tests the save() method on line 26, but it doesn't test the new start_branch parameter. Consider adding a test case that creates a file and saves it to a new branch using start_branch to ensure this fix works correctly.
JohnVillalovos
left a comment
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.
Thanks @nickbroon
LGTM
Passing
start_branchas kwargs results in it being passed as query argument to the API:which results in error being returned:
It should instead be sent a body attribute, which succeeds in creating the branch during the save.
To be sent as body attribute it must be specified as concrete function argument and class attribute instead of just using kwargs
Closes: #3318
Changes
Documentation and testing
Please consider whether this PR needs documentation and tests. This is not required, but highly appreciated: