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: Type correction on renaming file api #488

Merged
merged 4 commits into from Jun 23, 2022

Conversation

lizable
Copy link
Contributor

@lizable lizable commented Jun 23, 2022

Description

This PR resolves two things; setting the default value for is_dir parameter in the rename_file function, and providing more precise message in exception handling when virtual folder operation errors occurred on the same function.
I confirmed this issue during E2E testing for #443.

Screenshot(s)

Screen Shot 2022-06-23 at 12 16 18 PM

Screen Shot 2022-06-23 at 12 17 53 PM

@lizable lizable added type:bug Reports about that are not working comp:manager Related to Manager component effort:easy Need to understand only a specific region of codes (good first issue, easy). minor labels Jun 23, 2022
@lizable lizable added this to the 22.06 milestone Jun 23, 2022
@lizable lizable requested a review from achimnol June 23, 2022 03:54
@lizable lizable self-assigned this Jun 23, 2022
@@ -165,15 +164,18 @@ async def request(
try:
error_data = await client_resp.json()
raise VFolderOperationFailed(
extra_msg=error_data.pop("msg"),
Copy link
Member

Choose a reason for hiding this comment

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

👍🏼 Good catch!

@@ -485,7 +485,7 @@ async def rename_file(request: web.Request) -> web.Response:
t.Key("vfid"): tx.UUID(),
t.Key("relpath"): tx.PurePath(relative_only=True),
t.Key("new_name"): t.String(),
t.Key("is_dir"): t.ToBool(), # ignored since 22.03
Copy link
Member

Choose a reason for hiding this comment

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

You don't have to remove parenthesis of t.ToBool() because Trafaret automatically handles the difference of type reference and instance reference when there is no argument of a trafaret class.

@achimnol achimnol modified the milestones: 22.06, 22.03 Jun 23, 2022
@achimnol achimnol merged commit ae1c3c6 into main Jun 23, 2022
@achimnol achimnol deleted the fix/type-correction-on-renaming-file-API branch June 23, 2022 09:33
achimnol pushed a commit that referenced this pull request Jun 23, 2022
* fix: Set default value for `is_dir` param in renaming file function
* fix: Add exception handling on storage related request on manager component

Backported-From: main (22.06)
Backported-To: 22.03
@inureyes inureyes removed the minor label Apr 10, 2023
@Yaminyam Yaminyam added the size:S 10~30 LoC label Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:manager Related to Manager component effort:easy Need to understand only a specific region of codes (good first issue, easy). size:S 10~30 LoC type:bug Reports about that are not working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants