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

feat(pymilvus/settings.py): Load configuration without altering the environment #2192

Merged
merged 4 commits into from
Oct 8, 2024
Merged

feat(pymilvus/settings.py): Load configuration without altering the environment #2192

merged 4 commits into from
Oct 8, 2024

Conversation

laipz8200
Copy link
Contributor

related #2166.

This PR will replace environs with the more lightweight python-dotenv. It can load configuration from the dot env file without altering the system environment.

@sre-ci-robot
Copy link

Welcome @laipz8200! It looks like this is your first PR to milvus-io/pymilvus 🎉

@mergify mergify bot added the needs-dco label Jul 22, 2024
@laipz8200 laipz8200 force-pushed the feat/load-configuration-without-altering-the-environment branch from 8fa848a to 4854c64 Compare July 22, 2024 13:48
@mergify mergify bot added dco-passed and removed needs-dco labels Jul 22, 2024
XuanYang-cn
XuanYang-cn requested changes Jul 25, 2024
View reviewed changes
Copy link
Contributor

@XuanYang-cn XuanYang-cn left a comment

Choose a reason for hiding this comment

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

Please replace the dependencies in pyproject.toml too.

@laipz8200 laipz8200 requested a review from XuanYang-cn July 25, 2024 03:18
@laipz8200 laipz8200 force-pushed the feat/load-configuration-without-altering-the-environment branch from 47f605f to 61bd7ce Compare July 27, 2024 06:30
@mergify mergify bot added dco-passed and removed needs-dco labels Jul 27, 2024
@XuanYang-cn
Copy link
Contributor

/lgtm
/approve

@XuanYang-cn XuanYang-cn added the PR | need to cherry-pick to 2.x This PR need to be cherry-picked to 2.x branch label Jul 29, 2024
@laipz8200
Copy link
Contributor Author

Hello @XuanYang-cn, will this be merged? What should I do?

dr3s
dr3s suggested changes Aug 16, 2024
View reviewed changes
@dr3s
Copy link

dr3s commented Aug 16, 2024

Requested changes to ensure env vars set in other ways precede ones set in .env

@sre-ci-robot sre-ci-robot removed the lgtm label Aug 17, 2024
@mergify mergify bot added the needs-dco label Aug 17, 2024
@mergify mergify bot removed the dco-passed label Aug 17, 2024
@laipz8200 laipz8200 force-pushed the feat/load-configuration-without-altering-the-environment branch from 694be0f to d12b6e5 Compare August 17, 2024 01:19
@mergify mergify bot added dco-passed and removed needs-dco labels Aug 17, 2024
@laipz8200 laipz8200 requested a review from dr3s August 17, 2024 01:20
dr3s
dr3s reviewed Aug 17, 2024
View reviewed changes
dr3s
dr3s suggested changes Aug 17, 2024
View reviewed changes
Copy link

@dr3s dr3s left a comment

Choose a reason for hiding this comment

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

request to loosen version constraint

@laipz8200
chore(dependencies): Update python-dotenv dependency to allow version…
c74e5d5 
…s >=1.0.1 and <2.0.0 in pyproject.toml and requirements.txt.

Signed-off-by: -LAN- <[email protected]>
@laipz8200 laipz8200 requested a review from dr3s August 17, 2024 03:16
@dr3s
Copy link

dr3s commented Aug 17, 2024

LGTM!

@dr3s
Copy link

dr3s commented Aug 19, 2024

@XuanYang-cn can you approve again please?

The lgtm label was removed.

@laipz8200
Copy link
Contributor Author

Hello @XuanYang-cn! Could you please let me know when this change can be merged? Is there anything else I need to do?

@XuanYang-cn
Copy link
Contributor

Sorry about the late response!
/lgtm
/approve

@sre-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dr3s, laipz8200, XuanYang-cn

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot merged commit d255ef1 into milvus-io:master Oct 8, 2024
3 checks passed
@laipz8200 laipz8200 deleted the feat/load-configuration-without-altering-the-environment branch October 8, 2024 09:39
@laipz8200
Copy link
Contributor Author

@XuanYang-cn Thank you:)

hasansustcse13 pushed a commit to hasansustcse13/pymilvus that referenced this pull request Oct 9, 2024
@laipz8200
feat(pymilvus/settings.py): Load configuration without altering the e…
a75dac1 
…nvironment (milvus-io#2192)

related milvus-io#2166.

This PR will replace `environs` with the more lightweight
`python-dotenv`. It can load configuration from the dot env file without
altering the system environment.

---------

Signed-off-by: -LAN- <[email protected]>
Signed-off-by: hasan <[email protected]>
@XuanYang-cn XuanYang-cn added PR | cherry-picked to 2.x PR already cherry-picked to branch 2.x and removed PR | need to cherry-pick to 2.x This PR need to be cherry-picked to 2.x branch labels Nov 15, 2024
XuanYang-cn added a commit to XuanYang-cn/pymilvus that referenced this pull request Nov 15, 2024
@brunocfnba @laipz8200
@czs007 @CaoHaiNam @XuanYang-cn
Multi cherry picks from master branch
c497eb6 
- Added grpc as a valid protocol for uri (milvus-io#2090)
- build(deps): bump urllib3 from 1.26.18 to 1.26.19 (milvus-io#2140)
- build(deps): bump certifi from 2023.7.22 to 2024.7.4 (milvus-io#2170)
- feat(pymilvus/settings.py): Load configuration without altering the environment (milvus-io#2192)
- feat: Add compact, get_server_version and flush api (milvus-io#2326)
- Fix typo and correct grammar (milvus-io#2333)
- Update return type of describe_role to Dict (milvus-io#2337)
- enhance: Reorganize the examples (milvus-io#2340)

Related: milvus-io#2166, milvus-io#2325, milvus-io#2332

Co-authored-by: Bruno Faria <[email protected]>
Co-authored-by: Bruno Faria <[email protected]>
Co-authored-by: dependabot[bot] <[email protected]>
Co-authored-by: -LAN- <[email protected]>
Co-authored-by: zhenshan.cao <[email protected]>
Co-authored-by: NamCaoHai <[email protected]>
Co-authored-by: yangxuan <[email protected]>
@XuanYang-cn XuanYang-cn mentioned this pull request Nov 15, 2024
Merged
XuanYang-cn added a commit to XuanYang-cn/pymilvus that referenced this pull request Nov 15, 2024
@brunocfnba @laipz8200
@czs007 @CaoHaiNam @XuanYang-cn
Multi cherry picks from master branch
09dc873 
- Added grpc as a valid protocol for uri (milvus-io#2090)
- build(deps): bump urllib3 from 1.26.18 to 1.26.19 (milvus-io#2140)
- build(deps): bump certifi from 2023.7.22 to 2024.7.4 (milvus-io#2170)
- feat(pymilvus/settings.py): Load configuration without altering the environment (milvus-io#2192)
- feat: Add compact, get_server_version and flush api (milvus-io#2326)
- Fix typo and correct grammar (milvus-io#2333)
- Update return type of describe_role to Dict (milvus-io#2337)
- enhance: Reorganize the examples (milvus-io#2340)

Related: milvus-io#2166, milvus-io#2325, milvus-io#2332

Co-authored-by: Bruno Faria <[email protected]>
Co-authored-by: Bruno Faria <[email protected]>
Co-authored-by: dependabot[bot] <[email protected]>
Co-authored-by: -LAN- <[email protected]>
Co-authored-by: zhenshan.cao <[email protected]>
Co-authored-by: NamCaoHai <[email protected]>
Co-authored-by: yangxuan <[email protected]>
Signed-off-by: yangxuan <[email protected]>
sre-ci-robot pushed a commit that referenced this pull request Nov 25, 2024
@XuanYang-cn @brunocfnba
@laipz8200 @czs007 @CaoHaiNam
Multi cherry picks from master branch (#2349)
29bef71 
- Added grpc as a valid protocol for uri (#2090)
- build(deps): bump urllib3 from 1.26.18 to 1.26.19 (#2140)
- build(deps): bump certifi from 2023.7.22 to 2024.7.4 (#2170)
- feat(pymilvus/settings.py): Load configuration without altering the
environment (#2192)
- feat: Add compact, get_server_version and flush api (#2326)
- Fix typo and correct grammar (#2333)
- Update return type of describe_role to Dict (#2337)
- enhance: Reorganize the examples (#2340)

Related: #2166, #2325, #2332

Signed-off-by: yangxuan <[email protected]>
Co-authored-by: Bruno Faria <[email protected]>
Co-authored-by: Bruno Faria <[email protected]>
Co-authored-by: dependabot[bot] <[email protected]>
Co-authored-by: -LAN- <[email protected]>
Co-authored-by: zhenshan.cao <[email protected]>
Co-authored-by: NamCaoHai <[email protected]>
@NiuBlibing NiuBlibing mentioned this pull request Dec 4, 2024
Open
@sloria
Copy link

sloria commented Jan 13, 2025

fwiw, python-dotenv's load_env does alter os.environ. in fact, environs delegates to load_env, so this PR doesn't really change that.

i'm planning to add support for reading .env without altering os.environ sloria/environs#393 .

btw this isn't to argue againt this PR getting merged--i'm all for removing dependencies when sensible, even if they're my libraries 😅 . just raising that this PR may not have achieved its original intent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dr3s dr3s dr3s approved these changes

@XuanYang-cn XuanYang-cn XuanYang-cn approved these changes

@czs007 czs007 Awaiting requested review from czs007

@longjiquan longjiquan Awaiting requested review from longjiquan

Assignees

@longjiquan longjiquan

@XuanYang-cn XuanYang-cn

Labels
approved ci-passed dco-passed lgtm PR | cherry-picked to 2.x PR already cherry-picked to branch 2.x size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@laipz8200 @sre-ci-robot @XuanYang-cn @dr3s @sloria @longjiquan