fix: [sc-203919] Sharepoint plugin - Unable to read files whose names contain # character #66
Conversation
|
Not really related to the fix but there are issues with putting files in a new folder You cannot create an empty folder: But when you try to move to an non-existant folder, despite assurances it will be created, it does not like it:
|
|
Pre-existing probably, but the error display in some areas is not ideal. E.g. when you try to call a file something not allowed by sharepoint you get this:
|
There was a problem hiding this comment.
Mainly works fine, tested provider via connector and folder, these operations
- reading file
- folder paths creation and reading
- writing file
- renaming
- moving
Needs a small adjustment for move though (I know move is broken in the API, this is just to make it behave a bit less weird).
| # Using the new method leads to 403. | ||
| # Old method left in place. As a result, moving/renaming a file containing % in its path/name is still not possible. | ||
| # return self.get_file_url(from_path) + "/movetousingpath(newPath='{}',moveOperations=1)".format( | ||
| return self.get_file_url(from_path) + "/moveto(newurl='{}',flags=1)".format( | ||
| url_encode(self.get_site_path(to_path)) |
There was a problem hiding this comment.
It only seems to be moving a file into a folder with # or % that does not work.
You can rename files with these chars to ones without them (or still with them), or move them into folders without these chars. That generally works. But I think I know why this is different from your comment....
I noticed another problem - I think it is due to the new name value being url encoded (which i think you left in by accident). If rename something to name with a % this gets written in the file name as '%25'.
And there is no error - but the file seems to disappear (in the DSS folder UI), but it has had its name changed successfully - to one with %25, and appears again when the UI is refreshed.
So it would probably be best to remove the url_encode from line 726 for now - this will mean it does not work (gives an error) if the destination (folder or new name) has any of those characters, but does not half work or do anything really weird and confusing.
Screen.Recording.2024-09-30.at.18.53.52.mov
There was a problem hiding this comment.
After discussion, commit 62ba772 adds an assertion that no % is present in the to/from path of an item to move. This allows paths with # to be processed while failing with proper error message in other cases.
python-lib/sharepoint_client.py
Outdated
| assert_no_percent_in_path(full_from_path) | ||
| assert_no_percent_in_path(full_to_path) |
There was a problem hiding this comment.
Nit: might be better to put these in SharePointFSProvider with the other asserts? (But you can probably argue it both ways...)
There was a problem hiding this comment.
Done in 1cbae10
| @@ -168,6 +168,11 @@ def assert_valid_sharepoint_path(sharepoint_path): | |||
| ) | |||
|
|
|||
|
|
|||
| def assert_no_percent_in_path(path): | |||
| if isinstance(path, str) and "%" in path: | |||
| raise Exception("This plugin cannot move/rename an item if its path contains '%'") | |||
There was a problem hiding this comment.
This check applies if you add a '%' too so we need the error message to explain that case too I think?
This plugin cannot move/rename an item if its path contains '%', or to a path containing '%'"
There was a problem hiding this comment.
Because of the UI bug I'm worried about the size of the error message on most user's environments
Story details: https://app.shortcut.com/dataiku/story/203919