Refactor dotenv loading to use the dotenv package directly by bmerkle · Pull Request #179 · microsoft/typeagent-py

bmerkle · 2026-02-01T23:56:41Z

Refactor dotenv loading to use the dotenv package directly
removed function from utils.py
updated call sites
updated documentation AGENTS.md, docs/env-vars.md, and README.md

- Fixed embeddings.py: The issue was that when using OPENAI_API_KEY, the code was still looking up AZURE_OPENAI_ENDPOINT_EMBEDDING for the base URL. Now it correctly uses OPENAI_BASE_URL for standard OpenAI (which can be left unset for the default OpenAI API endpoint). - Fixed provider.py: sqllite transaction did not work on windows - release.py: just make format changes

- removed function from utils.py - updated call sites - updated documentation AGENTS.md, docs/env-vars.md, and README.md

Copilot

Pull request overview

This PR refactors environment-variable loading to use python-dotenv directly (via from dotenv import load_dotenv) and removes the custom utils.load_dotenv() helper, updating call sites and docs accordingly. It also includes additional changes to SQLite connection configuration and embedding endpoint configuration.

Changes:

Replace utils.load_dotenv() with dotenv.load_dotenv() across tools, tests, examples, and docs; remove the helper from src/typeagent/aitools/utils.py.
Change SQLite provider connection configuration to use isolation_level=None (autocommit) and add a test asserting the isolation level.
Refactor AsyncEmbeddingModel endpoint envvar configuration (renaming the endpoint_envvar concept and expanding it).

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tools/test_email.py	Switch env loading to `dotenv.load_dotenv()` in the email demo tool.
tools/query.py	Switch env loading to `dotenv.load_dotenv()` in the query tool.
tools/load_json.py	Switch env loading to `dotenv.load_dotenv()` in the JSON loader tool.
tools/ingest_vtt.py	Switch env loading to `dotenv.load_dotenv()` in the VTT ingest tool.
tools/ingest_podcast.py	Switch env loading import to `dotenv.load_dotenv()`.
tools/ingest_email.py	Switch env loading to `dotenv.load_dotenv()` in the email ingest tool.
tests/test_utils.py	Update dotenv-loading test to call `dotenv.load_dotenv()`.
tests/test_sqlitestore.py	Add test asserting SQLite connection isolation level is `None`.
tests/test_related_terms_index_population.py	Update dotenv import to `dotenv.load_dotenv()`.
tests/test_property_index_population.py	Update dotenv import to `dotenv.load_dotenv()`.
tests/test_message_text_index_population.py	Update dotenv import to `dotenv.load_dotenv()`.
tests/test_embeddings.py	Update tests for renamed embedding endpoint envvar configuration.
tests/conftest.py	Update auth-related fixtures to call `dotenv.load_dotenv()`.
src/typeagent/transcripts/README.md	Update docs snippet to use `dotenv.load_dotenv()`.
src/typeagent/storage/sqlite/provider.py	Change SQLite connect to autocommit (`isolation_level=None`).
src/typeagent/mcp/server.py	Switch env loading to `dotenv.load_dotenv()` in the server entrypoint.
src/typeagent/aitools/utils.py	Remove the custom `load_dotenv()` helper.
src/typeagent/aitools/embeddings.py	Refactor embedding endpoint envvar handling / rename `endpoint_envvar` concept.
examples/simple_query_demo.py	Switch env loading to `dotenv.load_dotenv()` in the demo.
examples/demo/query.py	Add `dotenv.load_dotenv()` usage in example (currently at import time).
examples/demo/ingest.py	Add `dotenv.load_dotenv()` usage in example (currently at import time).
docs/env-vars.md	Update documentation to reference `dotenv.load_dotenv()`.
AGENTS.md	Update contributor guidance to use `dotenv.load_dotenv()`.

Copilot · 2026-02-02T00:07:29Z

examples/demo/ingest.py

+from dotenv import load_dotenv
+
 from typeagent import create_conversation
 from typeagent.transcripts.transcript import TranscriptMessage, TranscriptMessageMeta

+load_dotenv()


load_dotenv() runs at import time, which can cause surprising side effects if this module is imported rather than executed. For consistency with other scripts in this repo (examples/simple_query_demo.py:22-26, tools/query.py:532-535), consider calling load_dotenv() inside main() or under the if __name__ == "__main__" guard instead.

Copilot · 2026-02-02T00:07:29Z

src/typeagent/storage/sqlite/provider.py

+        # Initialize database connection with autocommit mode
+        # isolation_level=None enables manual transaction control via BEGIN/COMMIT
+        self.db = sqlite3.connect(db_path, isolation_level=None)


PR description focuses on dotenv refactor, but this hunk also changes SQLite connection semantics (isolation_level=None / autocommit). Since this can affect transaction/rollback behavior and performance characteristics, it should be explicitly called out in the PR description (or split into a separate PR) so reviewers can assess the operational impact independently.

Copilot · 2026-02-02T00:07:30Z

docs/env-vars.md

 It is recommended to put your environment variables in a file named
 `.env` in the current or parent directory.
-To pick up these variables, call `typeagent.aitools.utils.load_dotenv()`
+To pick up these variables, call `load_dotenv()` from the `dotenv` package
 at the start of your program (before calling any typeagent functions).


This section tells users to call load_dotenv() but doesn’t show how to import it (or that it comes from the python-dotenv dependency). Consider adding a short explicit snippet like from dotenv import load_dotenv so readers can copy/paste without guessing the import path.

Copilot · 2026-02-02T00:07:30Z

examples/demo/query.py

+from dotenv import load_dotenv
+
 from typeagent import create_conversation
 from typeagent.transcripts.transcript import TranscriptMessage

+load_dotenv()


load_dotenv() is executed at import time here, which makes the module non-side-effect-free if it’s ever imported (e.g., by other examples/tests). Elsewhere in this repo, dotenv loading is done inside main() / the if __name__ == "__main__" block (e.g., examples/simple_query_demo.py:22-26, tools/ingest_podcast.py:13-65). Consider moving this call into main() or the __main__ guard for consistency.

gvanrossum · 2026-02-02T18:45:12Z

src/typeagent/aitools/embeddings.py

-    "text-embedding-3-small": (1536, "AZURE_OPENAI_ENDPOINT_EMBEDDING_3_SMALL"),
-    "text-embedding-3-large": (3072, "AZURE_OPENAI_ENDPOINT_EMBEDDING_3_LARGE"),
+# Map model names to (embedding_size, azure_endpoint_envvar, openai_endpoint_envvar)
+model_to_embedding_config: dict[str, tuple[int | None, str, str | None]] = {


It looks like the openai var is always None, because for openai we always use a default, DEFAULT_OPENAI_ENVVAR. Maybe the code can be mostly restored to what it was, and just use the latter in the openai case? I realize this may look like I'm retracting my previous comment, and I am -- I didn't realize that for openai we just always should use the same env var. I guess Azure is weird. :-)

Maybe we just rename the variables to suggested_azure_envvar and azure_endpoint_envvar. I'm not sure if we need to have the openai_endpoint_envvar argument, TBH.

gvanrossum · 2026-02-02T22:59:01Z

tests/test_sqlitestore.py

+    """Verify that the SQLite connection uses isolation_level=None for manual transaction control."""
+    # isolation_level=None enables autocommit mode, which allows manual
+    # transaction control via BEGIN/COMMIT/ROLLBACK statements
+    assert dummy_sqlite_storage_provider.db.isolation_level is None


Same comment I have in the "demo" PR -- this is not a real test IMO.

bmerkle · 2026-02-03T23:03:40Z

abort this PR

bmerkle added 6 commits February 1, 2026 18:31

Merge branch 'microsoft:main' into main

7f862be

Add test for SQLite connection isolation level in SqliteStorageProvider

25e28f2

Refactor embedding model environment variable handling for clarity

58a1e43

Merge branch 'main' into main

4279dc6

- Refactor dotenv loading to use the dotenv package directly

0af966e

- removed function from utils.py - updated call sites - updated documentation AGENTS.md, docs/env-vars.md, and README.md

Copilot AI review requested due to automatic review settings February 1, 2026 23:56

Copilot started reviewing on behalf of bmerkle February 1, 2026 23:57 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

bmerkle mentioned this pull request Feb 2, 2026

Fix demos for conf talk #181

Merged

gvanrossum reviewed Feb 2, 2026

View reviewed changes

bmerkle closed this Feb 3, 2026

bmerkle deleted the load-dotenv-lib branch February 3, 2026 23:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor dotenv loading to use the dotenv package directly#179

Refactor dotenv loading to use the dotenv package directly#179
bmerkle wants to merge 6 commits intomicrosoft:mainfrom
bmerkle:load-dotenv-lib

bmerkle commented Feb 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

Copilot AI Feb 2, 2026

Uh oh!

gvanrossum Feb 2, 2026

Uh oh!

gvanrossum Feb 2, 2026

Uh oh!

bmerkle commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bmerkle commented Feb 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

gvanrossum Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

gvanrossum Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

bmerkle commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants