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.

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(service): push to protected branches #1614

Merged
merged 4 commits into from Oct 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion conftest.py
Expand Up @@ -46,7 +46,7 @@
from tests.utils import make_dataset_add_payload

IT_PROTECTED_REMOTE_REPO_URL = os.getenv(
"IT_PROTECTED_REMOTE_REPO", "https://dev.renku.ch/gitlab/renku-qa/core-integration-test"
"IT_PROTECTED_REMOTE_REPO", "https://dev.renku.ch/gitlab/renku-qa/core-it-protected.git"
)

IT_REMOTE_REPO_URL = os.getenv("IT_REMOTE_REPOSITORY", "https://dev.renku.ch/gitlab/renku-qa/core-integration-test")
Expand Down Expand Up @@ -976,6 +976,9 @@ def svc_protected_repo(svc_client):

response = svc_client.post("/cache.project_clone", data=json.dumps(payload), headers=headers)

project_id = response.json["result"]["project_id"]
_ = svc_client.post("/cache.migrate", data=json.dumps(dict(project_id=project_id)), headers=headers)

yield svc_client, headers, payload, response


Expand Down
17 changes: 9 additions & 8 deletions renku/core/commands/save.py
Expand Up @@ -97,18 +97,19 @@ def repo_sync(repo, message=None, remote=None, paths=None):
raise errors.GitError("Cannot commit changes") from e

try:
# push local changes to remote branch
# NOTE: Push local changes to remote branch.
if origin.refs and repo.active_branch in origin.refs:
origin.fetch()
origin.pull(repo.active_branch)

origin.push(repo.active_branch)
result = origin.push(repo.active_branch)
if result and "[remote rejected] (pre-receive hook declined)" in result[0].summary:
Copy link
Member

Choose a reason for hiding this comment

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

It's a bit weird that git itself returns something like ! [remote rejected] master -> master (protected branch hook declined) but GitPython returns [remote rejected] (pre-receive hook declined) without the branch->branch part.

I wonder if there's a case where GitPython might also return the longer message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think GitPython only captures the last part of the message. It's also very strange that they don't raise exception on git error anymore but return the InfoResult object. 🤷

# NOTE: Push to new remote branch if original one is protected.
pushed_branch = uuid4().hex
repo.create_head(pushed_branch)
repo.remote().push(pushed_branch)

except git.exc.GitCommandError as e:
if "protected branches" not in e.stderr:
raise errors.GitError("Cannot push changes") from e
# push to new remote branch if original one is protected
pushed_branch = uuid4().hex
origin = repo.create_remote(pushed_branch, remote)
origin.push(repo.active_branch)
raise errors.GitError("Cannot push changes") from e

return saved_paths, pushed_branch
8 changes: 3 additions & 5 deletions tests/service/views/test_dataset_views.py
Expand Up @@ -945,7 +945,8 @@ def test_edit_datasets_view(svc_client_with_repo):
def test_protected_branch(svc_protected_repo):
"""Test adding a file to protected branch."""
svc_client, headers, payload, response = svc_protected_repo
assert response

assert_rpc_response(response)
assert {"result"} == set(response.json.keys())

payload = {
Expand All @@ -954,11 +955,8 @@ def test_protected_branch(svc_protected_repo):
}

response = svc_client.post("/datasets.create", data=json.dumps(payload), headers=headers,)
assert response

if "error" in response.json.keys() and response.json["error"]["migration_required"]:
# TODO: Fix this test to work with new project versions
return
assert_rpc_response(response)
assert {"result"} == set(response.json.keys())
assert "master" != response.json["result"]["remote_branch"]

Expand Down