FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix environment variable read issues and potential bugs by xiaoyongzhu · Pull Request #980 · feathr-ai/feathr · GitHub

/ feathr Public

Fix environment variable read issues and potential bugs - #980

Merged
xiaoyongzhu merged 4 commits into
feathr-ai:mainfrom
xiaoyongzhu:redis_client_fix
Jan 18, 2023
ConversationCommits (4)ChecksFiles changed
Merged

Fix environment variable read issues and potential bugs#980
xiaoyongzhu merged 4 commits into
feathr-ai:mainfrom
xiaoyongzhu:redis_client_fix

Conversation

xiaoyongzhu commented Jan 18, 2023

Copy link
Copy Markdown
Member

This PR fixes a few issues, mostly:

xiaoyongzhu mentioned this pull request Jan 18, 2023
Closed
xiaoyongzhu changed the title Close #976 Fix environment variable read issues and potential bugs Jan 18, 2023
Yuqing-cat reviewed Jan 18, 2023
View reviewed changes
Comment thread feathr_project/setup.py

blrchen commented Jan 18, 2023

Copy link
Copy Markdown
Collaborator

I see two changes in this PR

  1. Fix 976
  2. Left the pip deps version restriction

1 should be merged to main quickly as it's fixing issue on critial path. 2 might require more manual test and can take some time to get it merged. Can I suggest we split into two PRs so 1 can get merged quickly and make more room for 2 to get tested.

xiaoyongzhu commented Jan 18, 2023

Copy link
Copy Markdown
Member Author

I see two changes in this PR

  1. Fix 976
  2. Left the pip deps version restriction

1 should be merged to main quickly as it's fixing issue on critial path. 2 might require more manual test and can take some time to get it merged. Can I suggest we split into two PRs so 1 can get merged quickly and make more room for 2 to get tested.

Good point, this is reverted.

blrchen reviewed Jan 18, 2023
View reviewed changes
Comment thread feathr_project/feathr/client.py Outdated
blrchen approved these changes Jan 18, 2023
View reviewed changes

blrchen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

xiaoyongzhu merged commit a9ca048 into feathr-ai:main Jan 18, 2023
xiaoyongzhu added a commit to xiaoyongzhu/feathr that referenced this pull request Jan 19, 2023
Revert "Fix environment variable read issues and potential bugs (feat…
47baa4b 
…hr-ai#980)"

This reverts commit a9ca048.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

blrchen blrchen approved these changes
Yuqing-cat Yuqing-cat approved these changes
loomlike Awaiting requested review from loomlike
enya-yx Awaiting requested review from enya-yx
jainr Awaiting requested review from jainr
windoze Awaiting requested review from windoze

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL