Skip to content

Conversation

@omercnet
Copy link
Member

@omercnet omercnet commented Nov 10, 2024

Pull Request Summary: Migrate from requests to httpx

Overview

This PR modernizes the Descope Python SDK by migrating from the requests library to httpx, providing better async support, HTTP/2 capabilities, and improved performance for HTTP operations.

Key Changes

🔧 Core Infrastructure

  • Dependency Migration: Replaced requests with httpx ^0.27.2 in pyproject.toml and requirements.txt
  • HTTP Client Updates: Updated all HTTP operations in the core Auth class (descope/auth.py) to use httpx methods:
    • httpx.get(), httpx.post(), httpx.patch(), httpx.delete()
    • Updated parameter naming: allow_redirectsfollow_redirects
  • Public Key Fetching: Migrated _fetch_public_keys() method to use httpx.get()

📦 Auth Methods Updated

  • WebAuthn (descope/authmethod/webauthn.py): Updated import to use httpx.Response
  • All authentication flows: OTP, Password, SAML, SSO, TOTP, WebAuthn, EnchantedLink

🧪 Comprehensive Test Updates

Updated all test files to mock httpx instead of requests:

  • Core Tests: test_auth.py, test_descope_client.py
  • Auth Method Tests: test_otp.py, test_password.py, test_saml.py, test_sso.py, test_totp.py, test_webauthn.py, test_enchantedlink.py, test_magiclink.py, test_oauth.py
  • Management Tests: All management API tests (access keys, users, tenants, etc.)
  • Parameter Updates: Changed allow_redirects=False to follow_redirects=False in test assertions

🔄 Build & CI

  • Poetry Lock: Updated poetry.lock with new dependency resolution
  • Pre-commit: Fixed poetry export configuration in .pre-commit-config.yaml

Benefits

  • Modern HTTP Library: httpx provides better async support and HTTP/2 capabilities
  • Backward Compatibility: All public APIs remain unchanged - this is purely an internal implementation update
  • Performance: Potential performance improvements with better connection pooling and HTTP/2 support
  • Future-Proofing: httpx is actively maintained and designed for modern Python async patterns

Testing

  • ✅ All existing tests updated and passing
  • ✅ No breaking changes to public API
  • ✅ Comprehensive test coverage maintained across all authentication methods

Breaking Changes

None - this is an internal dependency migration that maintains full backward compatibility.

Files Changed

  • pyproject.toml - Updated dependency from requests to httpx
  • requirements.txt - Updated with new dependency resolution
  • poetry.lock - Updated lock file
  • descope/auth.py - Core HTTP client migration
  • descope/authmethod/webauthn.py - Import updates
  • All test files - Updated mocks and assertions
  • .pre-commit-config.yaml - Fixed poetry export

@github-actions
Copy link

github-actions bot commented Nov 10, 2024

Coverage report

The coverage rate went from 97.81% to 97.78% ⬇️

96% of new lines are covered.

Diff Coverage details (click to unfold)

descope/auth.py

94.44% of new lines are covered (95.56% of the complete file).
Missing lines: 125

descope/init.py

100% of new lines are covered (100% of the complete file).

descope/authmethod/enchantedlink.py

100% of new lines are covered (97.82% of the complete file).

descope/descope_client.py

100% of new lines are covered (97.75% of the complete file).

descope/authmethod/webauthn.py

100% of new lines are covered (100% of the complete file).

@omercnet omercnet enabled auto-merge (squash) November 10, 2024 11:27
@omercnet omercnet closed this Jun 26, 2025
auto-merge was automatically disabled June 26, 2025 18:30

Pull request was closed

@omercnet omercnet reopened this Jun 30, 2025
@ghost
Copy link

ghost commented Jun 30, 2025

Wiz Scan Summary

Displaying only findings that violated a policy

Scanner Findings
Vulnerability Finding Vulnerabilities
Data Finding Sensitive Data
Secret Finding Secrets
IaC Misconfiguration IaC Misconfigurations
Total

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

@ghost
Copy link

ghost commented Jun 30, 2025

Wiz Scan Summary

Displaying only findings that violated a policy

Scanner Findings
Vulnerability Finding Vulnerabilities
Data Finding Sensitive Data
Secret Finding Secrets
IaC Misconfiguration IaC Misconfigurations
Total

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

Copy link
Contributor

@LioriE LioriE left a comment

Choose a reason for hiding this comment

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

lgtm
what about liccheck.ini?
the last line there seems to be related to the requests dependency.

Replaced the 'requests' library with 'httpx' for all HTTP operations in the codebase, updated SSL context handling, and adjusted method signatures and usage accordingly. Updated dependencies in pyproject.toml and poetry.lock to include httpx and certifi. Also added missing __all__ to descope/__init__.py and created tests/testutils.py.
Copilot AI review requested due to automatic review settings October 5, 2025 07:39
Copy link
Contributor

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 modernizes the Descope Python SDK by migrating from the requests library to httpx, providing better async support, HTTP/2 capabilities, and improved performance. This is purely an internal implementation change that maintains full backward compatibility.

Key changes include:

  • Dependency migration from requests to httpx ^0.27.2 in configuration files
  • Core HTTP client updates to use httpx methods with parameter name changes (allow_redirectsfollow_redirects, okis_success)
  • Comprehensive test suite updates to mock httpx instead of requests

Reviewed Changes

Copilot reviewed 35 out of 36 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pyproject.toml Updated dependencies from requests to httpx
descope/descope_client.py Updated imports and return type annotations to use httpx
descope/authmethod/webauthn.py Updated import to use httpx.Response
descope/authmethod/enchantedlink.py Updated import from requests to httpx
tests/testutils.py Added SSLMatcher utility class for SSL context matching in tests
Various test files Updated mocks and assertions to use httpx instead of requests
samples/magiclink_web_sample_app.py Fixed Flask deprecation by replacing _request_ctx_stack with g

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

from ssl import SSLContext


class SSLMatcher:
Copy link

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

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

[nitpick] The SSLMatcher class lacks documentation explaining its purpose and usage. Consider adding a docstring to explain that this matcher is used in tests to verify SSL context parameters without exact value matching.

Suggested change
class SSLMatcher:
class SSLMatcher:
"""
Matcher used in tests to verify that an object is an SSLContext instance,
without requiring an exact value match. Useful for asserting that SSL context
parameters are set correctly in test cases.
"""

Copilot uses AI. Check for mistakes.
@@ -1,4 +1,4 @@
from flask import Flask, Response, _request_ctx_stack, jsonify, request
from flask import Flask, Response, g, jsonify, request
Copy link

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

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

The import change from _request_ctx_stack to g appears incomplete. The code uses g.top.claims but g is Flask's application context global, not a direct replacement for _request_ctx_stack.top. This should likely be just g.claims or use flask.g properly.

Copilot uses AI. Check for mistakes.
@descope_verify_magiclink_token(descope_client)
def verify_by_decorator(*args, **kwargs):
claims = _request_ctx_stack.top.claims
claims = g.top.claims
Copy link

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

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

The usage g.top.claims is incorrect. Flask's g object doesn't have a top attribute. This should likely be g.claims if the claims are stored directly on the Flask application context global.

Suggested change
claims = g.top.claims
claims = g.claims

Copilot uses AI. Check for mistakes.
@LioriE LioriE changed the base branch from main to feat/async-support October 5, 2025 08:32
@LioriE LioriE closed this Oct 5, 2025
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