Skip to content

Conversation

@julien-langjulien-langforce-pushed the ticket/SG-38306-python2-removal-httplib2 branch from 346dc35 to 6124681CompareJuly 17, 2025 16:27
@julien-langjulien-langforce-pushed the ticket/SG-38306-python2-various branch from 8ac9c8d to 5d8b18dCompareJuly 17, 2025 17:18
@julien-langjulien-lang requested a review from CopilotJuly 17, 2025 17:33
@julien-langjulien-lang marked this pull request as ready for review July 17, 2025 17:33
Copy link
Contributor

CopilotAI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR continues the Python 2 removal effort by cleaning up legacy Python 2/3 compatibility code, updating error handling, and modernizing imports.

  • Removed six.PY2/six.PY3 branches and outdated workarounds
  • Replaced sgsix.file_types with io.IOBase and ShotgunSSLError with ssl.SSLError
  • Simplified tests and CI configuration to target Python 3 only

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
FileDescription
tests/test_client.pyRemoved Python 2/3 guard around json.dumps; always call without encoding
tests/test_api.pyDropped ShotgunSSLError import, switched exception handling to ssl.SSLError
tests/base.pyRemoved Python 2-specific json.dumps(..., encoding="utf-8") branch
shotgun_api3/shotgun.pyUpdated imports (removed sgsix), added XML parsing in error handler, replaced sgsix.file_types with io.IOBase
azure-pipelines-templates/run-tests.ymlUpdated comments to reflect Python 3 versions only
README.mdCleaned up documentation by removing outdated Python 2/3 notes
Comments suppressed due to low confidence (2)

shotgun_api3/shotgun.py:2926

  • [nitpick] The new XML parsing branch in download_attachment error handling isn’t covered by existing tests. Consider adding a unit test that simulates an S3 XML error response to verify that the <Message> extraction works as intended.
 root = xml.etree.ElementTree.fromstring("".join(body)) 

tests/test_api.py:2259

  • The code now references ssl.SSLError but ssl is not imported in this file. Please add import ssl at the top of tests/test_api.py.
 mock_request.side_effect = ssl.SSLError( 

@julien-langjulien-lang requested a review from a teamJuly 17, 2025 17:34
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM. Just a couple of nitpicky suggestions.

Base automatically changed from ticket/SG-38306-python2-removal-httplib2 to masterSeptember 10, 2025 17:54
@eduardoChaucaGallegoseduardoChaucaGallegos dismissed stale reviews from carlos-villavicencio-adsk and themself September 10, 2025 17:54

The base branch was changed.

@eduardoChaucaGallegoseduardoChaucaGallegos merged commit 59b8f05 into masterSep 10, 2025
21 of 22 checks passed
@eduardoChaucaGallegoseduardoChaucaGallegos deleted the ticket/SG-38306-python2-various branch September 10, 2025 18:11
Sign up for freeto 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.

4 participants

@julien-lang@carlos-villavicencio-adsk@eduardoChaucaGallegos