Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[core] add HttpRequest and HttpResponse reprs #16972

Merged
merged 7 commits into from
Mar 5, 2021
Merged

[core] add HttpRequest and HttpResponse reprs #16972

merged 7 commits into from
Mar 5, 2021

Conversation

iscai-msft
Copy link
Contributor

fixes #16952

@ghost ghost added the Azure.Core label Feb 26, 2021
iscai-msft
iscai-msft commented Feb 26, 2021
View reviewed changes
sdk/core/azure-core/azure/core/pipeline/transport/_aiohttp.py Outdated
@@ -267,7 +267,7 @@ def __init__(self, request: HttpRequest, aiohttp_response: aiohttp.ClientRespons
self.status_code = aiohttp_response.status
self.headers = CIMultiDict(aiohttp_response.headers)
self.reason = aiohttp_response.reason
self.content_type = aiohttp_response.headers.get('content-type')
self.content_type = self.headers.get('content-type')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

callout: I think this should be self.headers instead of aiohttp_response.headers, bc we made self.headers case insensitive

Copy link
Member

Choose a reason for hiding this comment

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

Technically, from a functional perspective, this shouldn't matter since aiohttp_response.headers is also case-insensitive.

Copy link
Contributor Author

@iscai-msft iscai-msft Mar 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@lmazuel had brought that up too, I'll just revert this change

@iscai-msft
fix 2.7
bf96ec9
@iscai-msft
revert change to aiohttp
9159595
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
f66bdb4 
…into add_reprs

* 'master' of https://github.com/Azure/azure-sdk-for-python: (71 commits)
  EG - more docs imrpovement (Azure#17079)
  [EventHubs] add logging.info to warn the usage of partition key of non-string type (Azure#17057)
  march release (Azure#16966)
  Release of Device Update for IoT Hub SDK for Python. (Azure#17005)
  Add Get-AllPackageInfoFromRepo (Azure#16947)
  Track1 package is incorrectly set as track2 (Azure#17075)
  [text analytics] add normalized_text (Azure#17074)
  Renaming with_token identity function (Azure#17066)
  Adapt to azure core's cloud event (Azure#17063)
  align perf tests with js (Azure#17069)
  [Perfstress][Storage] Added FileShare perf tests (Azure#15834)
  [formrecognizer] Adding custom forms perf test (Azure#16969)
  Fix LanguageShort typo (Azure#17068)
  sas creds updates (Azure#17065)
  [eventgrid] Fix Sample eh (Azure#17064)
  [Perfstress][Storage] Added Datalake perf tests (Azure#15861)
  [text analytics] Healthcare n-ary relations (Azure#16997)
  ServiceBus dict-representation acceptance and kwarg-update functionality  (Azure#14807)
  [text analytics] add perf tests (Azure#17060)
  Add cloud event to core (Azure#16800)
  ...
johanste
johanste reviewed Mar 4, 2021
View reviewed changes
sdk/core/azure-core/azure/core/pipeline/transport/_base.py
@@ -592,6 +594,15 @@ def raise_for_status(self):
if self.status_code >= 400:
raise HttpResponseError(response=self)

def __repr__(self):
# there doesn't have to be a content type
content_type_str = (
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: I would exclude the content type key/value altogether in the repr if we don't know what it is (i.e. not add , Content-Type: to the output...

@iscai-msft iscai-msft requested a review from johanste March 4, 2021 21:02
@iscai-msft
Copy link
Contributor Author

/azp run python - core - ci

@iscai-msft
Copy link
Contributor Author

/azp run python - cosmos - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

kristapratico
kristapratico approved these changes Mar 5, 2021
View reviewed changes
Copy link
Member

@kristapratico kristapratico left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Verbal approval for this PR was given, I'm just approving so we can merge.

@iscai-msft
Copy link
Contributor Author

thanks @kristapratico , @johanste and @lmazuel have already given their verbal approval

@iscai-msft iscai-msft merged commit d728b2f into Azure:master Mar 5, 2021
@iscai-msft iscai-msft deleted the add_reprs branch March 5, 2021 17:38
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Mar 5, 2021
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
f297542 
…into assertions

* 'master' of https://github.com/Azure/azure-sdk-for-python:
  [core] add HttpRequest and HttpResponse reprs (Azure#16972)
  [text analytics] add actual normalized_text tests (Azure#17123)
  update samples to use actual role names (Azure#17124)
  Sync eng/common directory with azure-sdk-tools for PR 1448 (Azure#17085)
  Enable APIView status check (Azure#17107)
  Fix PackageName typo (Azure#17109)
  Move SetTestPipelineVersion.ps1 to eng/common (Azure#17103)
  Fix paths for non-windows agents (Azure#17096)
  [Communication] - Identity - Update README (Azure#17091)
  Rename CertificateCredential's certificate_bytes -> certificate_data (Azure#17090)
  fix shared reqs (Azure#17095)
  [translation] initial library (Azure#16837)
  EG - more docs imrpovement (Azure#17079)
  [EventHubs] add logging.info to warn the usage of partition key of non-string type (Azure#17057)
  march release (Azure#16966)
  Release of Device Update for IoT Hub SDK for Python. (Azure#17005)
  Add Get-AllPackageInfoFromRepo (Azure#16947)
  Track1 package is incorrectly set as track2 (Azure#17075)
iscai-msft added a commit to abhahn/azure-sdk-for-python that referenced this pull request Mar 5, 2021
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
a49dd8e 
…into ta-v5.1.0b6-analyze

* 'master' of https://github.com/Azure/azure-sdk-for-python: (24 commits)
  [text analytics] PII updates for v5.1.0b6 (Azure#17038)
  Fix bug where imported matrix parameter duplicates are not overrided (Azure#17126)
  Add NULL to readme (Azure#17076)
  Sas batching (Azure#17133)
  dropping py3.5 (Azure#17127)
  [EventHubs] 5.3.1 update changelog (Azure#17132)
  [text analytics] assertions (Azure#17098)
  add recordings (Azure#17125)
  [core] add HttpRequest and HttpResponse reprs (Azure#16972)
  [text analytics] add actual normalized_text tests (Azure#17123)
  update samples to use actual role names (Azure#17124)
  Sync eng/common directory with azure-sdk-tools for PR 1448 (Azure#17085)
  Enable APIView status check (Azure#17107)
  Fix PackageName typo (Azure#17109)
  Move SetTestPipelineVersion.ps1 to eng/common (Azure#17103)
  Fix paths for non-windows agents (Azure#17096)
  [Communication] - Identity - Update README (Azure#17091)
  Rename CertificateCredential's certificate_bytes -> certificate_data (Azure#17090)
  fix shared reqs (Azure#17095)
  [translation] initial library (Azure#16837)
  ...
@dependabot dependabot bot mentioned this pull request Mar 17, 2021
Merged
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-python that referenced this pull request Dec 9, 2021
@Amitbergman @sagamzu
@necoh @alondanoch @omerhaimov @omerhaimovich @msyyc
CodeGen from PR 16933 in Azure/azure-rest-api-specs
82a1f69 
Microsoft.SecurityInsights 2021-09-01-preview (Azure#16933)

* Adds base for updating Microsoft.SecurityInsights from version preview/2021-03-01-preview to version 2021-09-01-preview

* Updates readme

* Updates API version in new specs and examples

* Microsoft.security insights 2021 09 01 preview add missing resources (Azure#15531)

* Copy missing resources specs and examples from 2019-01-01-preview

* Update added resources specs and examples and extract common types

* Update readme

* Extract ClientInfo, UserInfo and Lable to common types

* Fix SpellCheck and Avocado

* Return ThreatIntelligence to readme

* Fix broken refs in Watchlists

* Resolve duplicate schema errors

* Run prettier

* Make common types prettier

* Add required property to operations according to ARM requirments

* Fix readme

* Add file separators to readme

* Rename example file

* Supress OBJECT_ADDITIONAL_PROPERTIES

* Add 'where' to OBJECT_ADDITIONAL_PROPERTIES supression

* Move OBJECT_ADDITIONAL_PROPERTIES supression under general Supression section.

* Copy dataConnectors from 2021-03-01-preview

* Update version of dataConnectors (this was done as there were errors when trying to generate C# client. Copying and changing version again fixed the problem).

* Add dataConnectorsCheckRequirments path, parameters and definitions from 2019-01-01-preveiw

Co-authored-by: Anat Gilenson <[email protected]>

* Use newest common types in new 2021-09-01-preview API version (Azure#15778)

* Use newest common types in AlertRules

* Use newest common types in AutomationRules

* Use newest common types in Bookmarks

* Use newest common types in dataConnectors

* Use newest common types in Enrichment

* Use newest common types in Entities

* Use newest common types in EntityQueries

* Use newest common types in Incidents

* Use newest common types in Metadata

* Use newest common types in OfficeConsents

* Use newest common types in OnboardingStates

* Use newest common types in operations

* Use newest common types in Settings

* Use newest common types in SourceControls

* Use newest common types in ThreatIntelligence

* Use newest common types in Watchlist

* Use newest common types in EntityTypes

* Use newest common types in RelationTypes

* Fix ThreatIntelligence

Co-authored-by: Anat Gilenson <[email protected]>

* Add template version to the scheduled alert rule + scheduled template (Azure#15919)

* Add template version to the scheduled alert rule

* Update AlertRules.json

* Update AlertRules.json

* Update AlertRules.json

* Update AlertRules.json

* Update GetAlertRuleTemplates.json

* Update GetAlertRuleTemplateById.json

* add aws s3 connector (Azure#15844)

* Add a new kind of alert rules - NRT (Azure#15980)

* add NRT rule

* add NRT rule

* add NRT rule

* add NRT rule

* fix typo

* fix typo

* fix

* Align new Metadata feature with 2021-03-01-preview (Azure#16304)

Co-authored-by: Anat Gilenson <[email protected]>

* Add fixes from 2021-03-01-preview (Azure#16238)

Co-authored-by: Anat Gilenson <[email protected]>

* Add entity query templates (Azure#16269)

* Add entity query templates from 2021-03-01-preview

* Update version

* Use newest common types and update readme

* Fix conflicting common types

Co-authored-by: Anat Gilenson <[email protected]>

* Fix bookmark relations operatinIds to be consistent with other operationIds. (Azure#16519)

Co-authored-by: Anat Gilenson <[email protected]>

* Add corrections from 2021-03-01-preview (Azure#16490)

Co-authored-by: Anat Gilenson <[email protected]>

* Remove unused parameters (Azure#16619)

Co-authored-by: Anat Gilenson <[email protected]>

* Update readme default readme tag for client generation (Azure#16620)

Co-authored-by: Anat Gilenson <[email protected]>

* Use CloudError instead of ErrorResponse to avoid breaking change (Azure#16691)

Co-authored-by: Anat Gilenson <[email protected]>

* Add data connectors polling ccp api support (Azure#16293)

* adding dataConnectors polling CCP api Support. (witout tests validations)

* azure sentinel dataconnectors update examples

* azure sentinel dataConnectors examples update and fix

* azure sentinel dataConnectors prettier

* azure sentinel dataConnectors add connect disconnect examples update path

* azure sentinel dataConnectors add connect disconnect examples fix

* azure sentinel dataConnectors add connect disconnect examples fix 2

* azure sentinel dataConnectors rebase dataConnectors dev

* azure sentinel dataconnectors - fix put to post on connect and disconnect endpoints

* azure sentinel dataconnectors - adding x-ms-secret to password on connect

* azure sentinel dataconnectors - connect/disconnect endpoint remove unnedded 201 return

* azure sentinel dataConnectors - remove empty body DataConnectorDisconnectBody

Co-authored-by: Alon Danoch <[email protected]>

* Add office IRM Connector (Azure#16764)

* Add office IRM

* fix

* fix

* fix

* fix

Co-authored-by: omerhaimov <[email protected]>

* Add teamInformation to IncidentProperties 2021-09-01-preview (Azure#16787)

* Fix Swagger for SecurityInsights - Add teamInformation to IncidentProperties

* Try change description as advised by Swagger reviewer Yuchao Yan to fix the validation error.

* Revert change in ntDomain description as it has nothing to do with validations

Co-authored-by: Anat Gilenson <[email protected]>

* Make CloudError and CloudErrorBody external resources (already exist under Microsoft.Rest.Azure namespace) (Azure#16872)

Co-authored-by: Anat Gilenson <[email protected]>

* Remove operational insights parameter 2021 09 01 preview (Azure#16891)

* Remove operationalInsightsResourceProvider parameter from specs

* Remove parameter from examples

Co-authored-by: Anat Gilenson <[email protected]>

* Update EntityTypes.json (Azure#16972)

Co-authored-by: Anat Gilenson <[email protected]>
Co-authored-by: Amit Bergman <[email protected]>
Co-authored-by: sagamzu <[email protected]>
Co-authored-by: necoh <[email protected]>
Co-authored-by: alondanoch <[email protected]>
Co-authored-by: Alon Danoch <[email protected]>
Co-authored-by: omerhaimov <[email protected]>
Co-authored-by: omerhaimov <[email protected]>
Co-authored-by: Yuchao Yan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiangyan99 xiangyan99 xiangyan99 approved these changes

@kristapratico kristapratico kristapratico approved these changes

@lmazuel lmazuel Awaiting requested review from lmazuel

@annatisch annatisch Awaiting requested review from annatisch

@johanste johanste Awaiting requested review from johanste

Assignees
No one assigned
Labels
Azure.Core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[core] add good reprs for our HttpRequest and HttpResponses
4 participants
@iscai-msft @xiangyan99 @johanste @kristapratico