Skip to content

Cloud info - use same session #588

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

Merged
merged 3 commits into from
Jun 24, 2025
Merged

Cloud info - use same session #588

merged 3 commits into from
Jun 24, 2025

Conversation

AsafMah
Copy link
Collaborator

@AsafMah AsafMah commented Jun 22, 2025

No description provided.

Copilot

This comment was marked as outdated.

Copy link

github-actions bot commented Jun 22, 2025

Test Results

    6 files  ±0      6 suites  ±0   20m 16s ⏱️ + 2m 25s
  317 tests ±0    282 ✅ ±0   35 💤 ±0  0 ❌ ±0 
1 902 runs  ±0  1 692 ✅ ±0  210 💤 ±0  0 ❌ ±0 

Results for commit fa64178. ± Comparison against base commit d5fcedf.

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Jun 22, 2025

Codecov Report

Attention: Patch coverage is 83.33333% with 5 lines in your changes missing coverage. Please review.

Project coverage is 88.96%. Comparing base (d72e890) to head (fa64178).
Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
...ure-kusto-data/azure/kusto/data/_cloud_settings.py 75.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #588      +/-   ##
==========================================
- Coverage   89.13%   88.96%   -0.17%     
==========================================
  Files          44       44              
  Lines        3763     3779      +16     
==========================================
+ Hits         3354     3362       +8     
- Misses        409      417       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AsafMah AsafMah requested a review from Copilot June 24, 2025 06:53
Copy link

@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 ensures that the same HTTP session is used consistently in both ingestion and query operations, improving connection reuse and performance. Key changes include a documentation fix for the DataFormat parameter, updates to use requests.Session for HTTP calls in tests and client code, and modifications to accept an injected session in cloud info token providers and cloud settings.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
azure-kusto-ingest/azure/kusto/ingest/base_ingest_client.py Corrected a typo in the DataFormat documentation.
azure-kusto-data/tests/test_kusto_client.py Updated the patch decorator to use requests.Session.get for consistency.
azure-kusto-data/azure/kusto/data/client_base.py Added a _session attribute and ensured the token provider uses the session.
azure-kusto-data/azure/kusto/data/_token_providers.py Allowed session injection in token providers.
azure-kusto-data/azure/kusto/data/_cloud_settings.py Modified get_cloud_info_for_cluster to accept an optional session.
CHANGELOG.md Updated to reflect the improved session reuse.
Comments suppressed due to low confidence (1)

azure-kusto-data/azure/kusto/data/_cloud_settings.py:59

  • [nitpick] Consider updating the docstring for get_cloud_info_for_cluster to include details on the session parameter and its expected usage to improve clarity for future maintainers.
    def get_cloud_info_for_cluster(cls, kusto_uri: str, proxies: Optional[Dict[str, str]] = None, session: requests.Session = None) -> CloudInfo:

@AsafMah AsafMah merged commit 69825fc into master Jun 24, 2025
13 checks passed
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.

2 participants