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

lakectl to present error while writing to protected branch #8454

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ItamarYuran
Copy link
Contributor

Closes #8333

@ItamarYuran ItamarYuran added the exclude-changelog PR description should not be included in next release changelog label Jan 2, 2025
@ItamarYuran ItamarYuran marked this pull request as draft January 2, 2025 14:06
Copy link

github-actions bot commented Jan 2, 2025

🎊 PR Preview edd426d has been successfully built and deployed to https://treeverse-lakeFS-preview-pr-8454.surge.sh

🕐 Build time: 0.012s

🤖 By surge-preview

Copy link

github-actions bot commented Jan 2, 2025

E2E Test Results - DynamoDB Local - Local Block Adapter

13 passed

Copy link

github-actions bot commented Jan 2, 2025

E2E Test Results - Quickstart

11 passed

@ItamarYuran ItamarYuran force-pushed the 8333-bug-lakectl-local-doesnt-mention-the-reason-commit-fails-when-writing-to-a-protected-branch branch from 4f43a89 to 33ca670 Compare January 2, 2025 14:53
@ItamarYuran ItamarYuran requested a review from idanovo January 5, 2025 10:24
@idanovo
Copy link
Contributor

idanovo commented Jan 5, 2025

@ItamarYuran please move the PR to ready for review.
And please avoid next time from creating branches with 90 chars length 😅

@ItamarYuran ItamarYuran marked this pull request as ready for review January 5, 2025 11:27
Copy link
Contributor

@idanovo idanovo left a comment

Choose a reason for hiding this comment

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

There are a couple of issues with the current solution:

  1. 403 (forbidden) status code might be a result of another issue than writing to a protected branch.
  2. Your solution works specifically for uploading an object with presign mode without using multiparts.
    What if I delete an object instead of uploading one? I'll still have the same issue.
  3. I would like to have some tests here to validate we show the reason why we failed when trying to apply changes (write/delete) to a protected branch.

@ItamarYuran ItamarYuran force-pushed the 8333-bug-lakectl-local-doesnt-mention-the-reason-commit-fails-when-writing-to-a-protected-branch branch from c3eeadb to cce44f3 Compare January 7, 2025 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude-changelog PR description should not be included in next release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: lakectl local doesn't mention the reason commit fails when writing to a protected branch
2 participants