Skip to content

Conversation

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

@Copilot Copilot AI 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
File Description
tests/test_client.py Removed Python 2/3 guard around json.dumps; always call without encoding
tests/test_api.py Dropped ShotgunSSLError import, switched exception handling to ssl.SSLError
tests/base.py Removed Python 2-specific json.dumps(..., encoding="utf-8") branch
shotgun_api3/shotgun.py Updated imports (removed sgsix), added XML parsing in error handler, replaced sgsix.file_types with io.IOBase
azure-pipelines-templates/run-tests.yml Updated comments to reflect Python 3 versions only
README.md Cleaned 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-lang julien-lang requested a review from a team July 17, 2025 17:34
Copy link
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. Just a couple of nitpicky suggestions.

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

The base branch was changed.

@eduardoChaucaGallegos eduardoChaucaGallegos merged commit 59b8f05 into master Sep 10, 2025
21 of 22 checks passed
@eduardoChaucaGallegos eduardoChaucaGallegos deleted the ticket/SG-38306-python2-various branch September 10, 2025 18:11
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