- Notifications
You must be signed in to change notification settings - Fork94
Handle NotImplementedError raised by acl_win#1077
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ikus060 commentedJun 26, 2025
@ericzolf If you could tkae a look at this one. I did not write unit test as I dont see any specific test for acl_win... |
ericzolf commentedJun 27, 2025
I made this PR a draft and removed the WIP prefix, you old GitLab afficionado! 😏 More seriously:
|
ikus060 commentedJun 27, 2025
@ericzolf The git diff looks a bit weird, but honestly, I didn't change much. I just moved most of the code interacting with the DACL into a try...except block and added a new exception type to catch: NotImplementedError. This is all to handle an exception raised by acl.GetAce() when it encounters an unsupported ACE type: |
ericzolf commentedJun 28, 2025
My comment was definitely more about my foggy brain that about your change. |
Signed-off-by: Patrik Dufresne <patrik@ikus-soft.com>
749d06b toe1c3078Compare
ericzolf 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.
I allowed myself to rebase your branch to make the comparison easier locally.
The code itself looks all good to me. Two little things:
- change the commit message to add a FIX explaining the change
- would you mind creating a new test file for win_acl and adding a test (or two)?
ericzolf commentedJul 11, 2025
Out of curiosity, why did you close this PR? |
ikus060 commentedJul 11, 2025
I might have delete the branch my mistake. |
Changes done and why
Change error handling arround acl_win.py to catch
NotImplementedErrorthat get raised byacl.GetAce()when the ACL type is not supported.This issue was repported by a Minarca User.
https://gitlab.com/ikus-soft/minarca/-/issues/324
Self-Checklist