Skip to content

SG-34551 validation for python3, so display_name now is only decoded …#337

Merged
eduardoChaucaGallegos merged 6 commits into
masterfrom
ticket/SG-34551-api-adds-prefix-and-sufix
Jun 6, 2024
Merged

SG-34551 validation for python3, so display_name now is only decoded …#337
eduardoChaucaGallegos merged 6 commits into
masterfrom
ticket/SG-34551-api-adds-prefix-and-sufix

Conversation

@eduardoChaucaGallegos
Copy link
Copy Markdown
Contributor

Now prefix/suffix if display_name property exists is not added

…for python3 and python2 keeps working in same way
Comment thread shotgun_api3/shotgun.py Outdated
Copy link
Copy Markdown
Member

@julien-lang julien-lang left a comment

Choose a reason for hiding this comment

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

Looks good. Are there any other fields that could have the same problem?

Also is it possible to add unit tests?

@eduardoChaucaGallegos
Copy link
Copy Markdown
Contributor Author

@julien-lang By the moment I couldn't see other fields with that potential problem in the scope of "_upload_to_sg" function because it is the only one field that previously was encoded to utf-8(encode("utf-8")) to assurance the behavior of non-ascii unicode and utf-8 string paths during upload.
Also I believe that is not necessary a new test for that new condition because that function and complete upload flow already has a unit tests. There exists a possibility to use another python2 "runner" in settings of CI/CD, but If I'm not wrong, we only will support python3 in the next weeks

Comment thread tests/test_api.py Outdated
Comment thread tests/test_api.py
'attachments',
tag_list="monkeys, everywhere, send, help"
)
self.assertTrue(isinstance(upload_id, int))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you have access to mock_send_form.call_args? I'm wondering if we can inspect if params["thumb_image"] and params["file"] values when calling _send_form are correct without unwanted prefix and suffix.

Comment thread tests/test_api.py Outdated
Comment thread tests/test_api.py Outdated
Copy link
Copy Markdown
Contributor

@carlos-villavicencio-adsk carlos-villavicencio-adsk left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment thread shotgun_api3/shotgun.py Outdated
if six.PY2:
display_name = os.path.basename(path)
else:
display_name = os.path.basename(path.decode("utf-8"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you 15 lines above. There is a big comment about unicode path and we encode path in utf-8... Looks like the opposite of what's done here. We might want to refactor/clean/review all that

Copy link
Copy Markdown
Member

@julien-lang julien-lang left a comment

Choose a reason for hiding this comment

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

LGTM!

@eduardoChaucaGallegos eduardoChaucaGallegos merged commit 2e1cd50 into master Jun 6, 2024
@eduardoChaucaGallegos eduardoChaucaGallegos deleted the ticket/SG-34551-api-adds-prefix-and-sufix branch June 6, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants