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

Update token exchange environment variable name #20506

Merged
merged 1 commit into from
Sep 2, 2021
Merged

Update token exchange environment variable name #20506

merged 1 commit into from
Sep 2, 2021

Conversation

chlowell
Copy link
Member

@chlowell chlowell commented Sep 1, 2021

TOKEN_FILE_PATH -> AZURE_FEDERATED_TOKEN_FILE

FYI @christothes @g2vinay @sadasant

@ghost ghost added the Azure.Identity label Sep 1, 2021
xiangyan99
xiangyan99 reviewed Sep 1, 2021
View reviewed changes
sdk/identity/azure-identity/azure/identity/_constants.py
@@ -47,5 +47,5 @@ class EnvironmentVariables:
AZURE_IDENTITY_ENABLE_LEGACY_TENANT_SELECTION = "AZURE_IDENTITY_ENABLE_LEGACY_TENANT_SELECTION"
AZURE_REGIONAL_AUTHORITY_NAME = "AZURE_REGIONAL_AUTHORITY_NAME"

TOKEN_FILE_PATH = "TOKEN_FILE_PATH"
TOKEN_EXCHANGE_VARS = (AZURE_CLIENT_ID, AZURE_TENANT_ID, TOKEN_FILE_PATH)
AZURE_FEDERATED_TOKEN_FILE = "AZURE_FEDERATED_TOKEN_FILE"
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we use TOKEN_FILE_PATH = "AZURE_FEDERATED_TOKEN_FILE" so it can be less breaking?

Copy link
Member Author

Choose a reason for hiding this comment

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

By "be less breaking" do you mean "replacing fewer references" (there's no API to break, this is an internal constant)? Having the constant match its value seems more important to me.

Copy link
Member

Choose a reason for hiding this comment

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

My understanding is if we keep TOKEN_FILE_PATH , we don't need to update those in tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

That makes the diff of this PR smaller, but then the constant doesn't match its value. Given its meaning and how it's used, that seems more important to me.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds fair.

@chlowell chlowell merged commit 9598c3f into Azure:main Sep 2, 2021
@chlowell chlowell deleted the federate branch September 2, 2021 16:05
iscai-msft pushed a commit that referenced this pull request Sep 29, 2021
@chlowell @iscai-msft
Update token exchange environment variable name (#20506)
e84fb6e
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-python that referenced this pull request Oct 3, 2022
@namanag16
CodeGen from PR 20800 in Azure/azure-rest-api-specs
5cd8f2d 
[Hub Generated] Review request for Microsoft.MachineLearningServices to add version stable/2022-10-01 (Azure#20800)

* Adds base for updating Microsoft.MachineLearningServices from version stable/2022-05-01 to version 2022-10-01

* Updates readme

* Updates API version in new specs and examples

* Added readOnly property (Azure#20506)

* Added readOnly property

* Added changes to sync with main

* Fixed Prettier check

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

* Add schedule related schemas for Compute Instance (Azure#20697)

* support for update schedules on Compute Instance

* change put to patch

* remove update schedules rest call

Co-authored-by: Naman Agarwal <[email protected]>

* Added mfe.json changes (Azure#20514)

* Added mfe.json changes

* Added checks related changes

* Updated mfe.json to handle online endpoint and deployment changes

* Updated mfe.json with AutoRebuild changes

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

* Removed duplicate definitions and updated with mfe references (Azure#20840)

* Removed duplicate definitions and updated with mfe references

* Replaced Cron to CronTrigger, Recurrence to RecurrenceTrigger from mfe

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

Co-authored-by: maulikjoshi <[email protected]>
Co-authored-by: Naman Agarwal <[email protected]>
Co-authored-by: Naman Agarwal <[email protected]>
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-python that referenced this pull request Oct 5, 2022
@fahdkmsft @namanag16
@srivatsasinha @ZhidaLiu @prakharg-msft @forteddyt
CodeGen from PR 20818 in Azure/azure-rest-api-specs
214a3b1 
[Hub Generated] Review request for Microsoft.MachineLearningServices to add version preview/2022-10-01-preview (Azure#20818)

* Adds base for updating Microsoft.MachineLearningServices from version preview/2022-06-01-preview to version 2022-10-01-preview

* Updates readme

* Updates API version in new specs and examples

* Registries public preview 2022-10-01-preview (Azure#20200)

* Registries public preview 2022-10-01-preview

* Fix validation errors

* Fix LintDiff and Avocado errors

* Fixing prettier errors

* Fixing more LintDiff errors

* Another LintDiff error

* Another Lint error

Co-authored-by: Fahd Kamal <[email protected]>

* Added schedule api (Azure#20273)

* added schedule api

* add updateSchedules to examples parent folder

* fix typo

* change position of allOf

* remove subnet from example compute create

* after prettier run

* Description change for recurrence freq

* point to mfe for common objects

* add hash operator

* removed triggertype as well

* change year

* get rid of breaking changes since we didn't change the api

Co-authored-by: Naman Agarwal <[email protected]>

* Added Compute Instance OS Patching Properties (Azure#20284)

* Added CI Os Patching related properties

* Updated descriptions as per Dennis's suggestions

Co-authored-by: Srivatsa Sinha <[email protected]>

* Maulik/sync preview (Azure#20554)

* Added changes to sync with main

* Fixed Prettier Check

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

* Remove update schedules REST call for compute instance (Azure#20699)

* change post call to put

* change put to patch

* remove updateschedules rest call

Co-authored-by: Naman Agarwal <[email protected]>

* Update connection type and auth type (Azure#20493)

* Update connection type and auth type

* update connection category

* update x-ms-secret tag

* Add autologger_settings for CI (Azure#20302)

* Add autologger_settings for CI

Add settings to opt-in/out of default mlflow autologger on CIs.

* update machinelearningservices.json

* Add for command job

* fix comma

* Add autologger for command job

* fix CI failures

* Remove mfe.json from PR

* Remove whitespace

* Update custom-words.txt

* Delete incorrect custom-words.txt

* Fixing auto-rest issues in registries (Azure#20772)

* Fixing auto-rest issues in registries

* Removing location from the required list

Co-authored-by: Fahd Kamal <[email protected]>

* Maulik/mfe preview changes (Azure#20372)

* Added mfe.json preview changes

* Added resources to Registries and Workspaces folder

* removed unwanted resources

* removed unwanted resources

* Ran prettier check

* removed unwanted resources

* removed unwanted resources

* Added ManagedOnlineDeployment resource

* Added files to OnlineDeplyment

* Added Model Validation changes

* Added mfe.json changes to handle duplicate operation id changes

* Updated mfe to fix underscore error

* Update title of mfe.json

* Renamed name path parameter

* Updated examples

* Fixed Semantic validations and examples

* Fixed Semantic validations and examples

* Fixed Semantic validations and examples

* Removed name changes from workspaces

* Changes related to Model Validation

* Changes related to Model validation

* Added consumes amd produces in registries.json

* Removed changes from registries

* Updated parameter mlFlowAutologger to mlflowAutologger in mfe.json

* Changed property name to mlflowAutologger

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

* Updated custom words to resolve merge conflict (Azure#20833)

* Updated custom words to resolve merge conflict

* Updated custom words to resolve merge conflict

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

* Resolved merge conflict (Azure#20845)

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

* Fixed LintDiff checks (Azure#20847)

* Fixed LintDiff checks

* Fixed LintDiff checks

* Fixed LintDiff checks

* Fixed LintDiff checks

* Updated examples

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

* Resolved merge conflict of cutsom-words.txt (Azure#20956)

* Resolved merge conflict

* Resolved merge conflict

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

* [Hub Generated] Review request for Microsoft.MachineLearningServices to add version stable/2022-10-01 (Azure#20800) (Azure#20982)

* Adds base for updating Microsoft.MachineLearningServices from version stable/2022-05-01 to version 2022-10-01

* Updates readme

* Updates API version in new specs and examples

* Added readOnly property (Azure#20506)

* Added readOnly property

* Added changes to sync with main

* Fixed Prettier check

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

* Add schedule related schemas for Compute Instance (Azure#20697)

* support for update schedules on Compute Instance

* change put to patch

* remove update schedules rest call

Co-authored-by: Naman Agarwal <[email protected]>

* Added mfe.json changes (Azure#20514)

* Added mfe.json changes

* Added checks related changes

* Updated mfe.json to handle online endpoint and deployment changes

* Updated mfe.json with AutoRebuild changes

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

* Removed duplicate definitions and updated with mfe references (Azure#20840)

* Removed duplicate definitions and updated with mfe references

* Replaced Cron to CronTrigger, Recurrence to RecurrenceTrigger from mfe

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

Co-authored-by: maulikjoshi <[email protected]>
Co-authored-by: Naman Agarwal <[email protected]>
Co-authored-by: Naman Agarwal <[email protected]>

Co-authored-by: maulikjoshi <[email protected]>
Co-authored-by: Naman Agarwal <[email protected]>
Co-authored-by: Naman Agarwal <[email protected]>

* Revert "[Hub Generated] Review request for Microsoft.MachineLearningS… (Azure#20983)

* Revert "[Hub Generated] Review request for Microsoft.MachineLearningServices to add version stable/2022-10-01 (Azure#20800) (Azure#20982)"

This reverts commit 649ef8334aca13558e534babdcf53cef6831f22a.

* Updated readme.md to resolve merge conflict

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

* Maulik/fix preview (Azure#20994)

* Merged readme from main

* Added preview tag changes

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

* shuffle readme tags (Azure#21005)

* conflict resolution (Azure#21007)

Co-authored-by: fahdkmsft <[email protected]>
Co-authored-by: Fahd Kamal <[email protected]>
Co-authored-by: Naman Agarwal <[email protected]>
Co-authored-by: Naman Agarwal <[email protected]>
Co-authored-by: srivatsasinha <[email protected]>
Co-authored-by: Srivatsa Sinha <[email protected]>
Co-authored-by: maulikjoshi <[email protected]>
Co-authored-by: ZhidaLiu <[email protected]>
Co-authored-by: prakharg-msft <[email protected]>
Co-authored-by: Teddy Todorov <[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

@mccoyp mccoyp mccoyp approved these changes

@schaabs schaabs Awaiting requested review from schaabs

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

Successfully merging this pull request may close these issues.
3 participants
@chlowell @xiangyan99 @mccoyp