Skip to content

minor update for ai-service#493

Merged
paopa merged 5 commits intomainfrom
chore/ai-service/rewrite-env-url
Jul 9, 2024
Merged

minor update for ai-service#493
paopa merged 5 commits intomainfrom
chore/ai-service/rewrite-env-url

Conversation

@cyyeh
Copy link
Member

@cyyeh cyyeh commented Jul 8, 2024

  • automatically remove trailing / for urls in env vars
  • fix azure open ai llm issue
  • fix auto redeploy issue

@cyyeh cyyeh added the module/ai-service ai-service related label Jul 8, 2024
@cyyeh cyyeh requested a review from paopa July 8, 2024 10:48
@cyyeh cyyeh changed the title remove trailing / in urls of env vars minor update for ai-service Jul 9, 2024
Copy link
Contributor

@paopa paopa left a comment

Choose a reason for hiding this comment

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

Thanks for your hard work! I have left two comments with my thoughts.

Comment on lines 8 to 10

load_dotenv(override=True)
if is_dev_env := os.getenv("ENV") and os.getenv("ENV").lower() == "dev":
if Path(".env.dev").exists():
load_dotenv(".env.dev", override=True)
Copy link
Contributor

@paopa paopa Jul 9, 2024

Choose a reason for hiding this comment

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

what do you think using utils.load_env_vars func to instead of this.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw many place need checking and removing the slash at the end of an endpoint. I think we could extract them a func in utils module.

def endpoint_formatter(endpoint: str) -> str:
    return endpoint.rstrip("/") if endpoint.endswith("/") else endpoint

Copy link
Member Author

Choose a reason for hiding this comment

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

done, and the function name is remove_trailing_slash

Copy link
Contributor

@paopa paopa left a comment

Choose a reason for hiding this comment

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

LGTM

@paopa paopa merged commit 6194a10 into main Jul 9, 2024
@paopa paopa deleted the chore/ai-service/rewrite-env-url branch July 9, 2024 04:04
onlyjackfrost pushed a commit that referenced this pull request Jul 11, 2024
* remove trailing / in urls of env vars

* update

* fix

* fix bug

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

Labels

module/ai-service ai-service related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants