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.

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

fix regression where COLD_PW can no longer be passed with env vars co… #2308

Conversation

mjurbanski-reef
Copy link
Contributor

@mjurbanski-reef mjurbanski-reef commented Sep 14, 2024

Bug

Description of the Change

Apparently, in the context of python it was possible to set a env var with hyphen in it. Which makes change introduced by #1949 a breaking one which is not acceptable as it was introduced in minor version of bittensor, hence breaking semver contract.

This PR makes sure we support proper env variable names, while also not breaking users that used hyphens within the env var name.

See https://discord.com/channels/1120750674595024897/1242999357436071956/1284309400751964271 for more context.

Alternate Designs

Ignore the regression, but that conflicts with semver versioning and further discourages keeping with the latest bittensor version.

Possible Drawbacks

Not forcing users to migrate asap.

Verification Process

Unit test

Release Notes

  • fix regression which broke passing passcode through env var with hyphen, e.g. BT_COLD_PW_My-Wallet3=pw. Please note this form is deprecated, and one should use BT_COLD_PW_My_Wallet3=pw instead.

Copy link

@alex-drocks alex-drocks left a comment

Choose a reason for hiding this comment

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

Looks good to me 🙏

}

monkeypatch.setenv("bt_cold_pw_wallet", password_by_wallet["WALLET"])
monkeypatch.setenv("BT_COLD_PW_My_Wallet", password_by_wallet["my_wallet"])
monkeypatch.setenv("BT_COLD_PW_My-Wallet3", password_by_wallet["my-wallet3"])
Copy link
Contributor

Choose a reason for hiding this comment

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

won't the new warning crash this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CI is passing. Test are not run with pytest -W error if this is what you are referring to.

Or did I misunderstand you and you mean something else?

Co-authored-by: garrett-opentensor <[email protected]>
@mjurbanski-reef
Copy link
Contributor Author

no longer relevant after refactor

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.

7 participants