feat: Remove initialize() Method from LlamaStackAsLibrary #2979

Elbehery · 2025-07-31T12:20:26Z

What does this PR do?

This PR removes init() from LlamaStackAsLibrary

Currently client.initialize() had to be invoked by user.
To improve dev experience and to avoid runtime errors, this PR init LlamaStackAsLibrary implicitly upon using the client.
It prevents also multiple init of the same client, while maintaining backward ccompatibility.

This PR does the following

Automatic Initialization: Constructor calls initialize_impl() automatically.
Client is fully initialized after init completes.
Prevents consecutive initialization after the client has been successfully initialized.
initialize() method still exists but is now a no-op.

fixes #2946

Elbehery · 2025-07-31T12:20:46Z

cc @mattf @nathan-weinberg

nathan-weinberg · 2025-07-31T13:37:36Z

Code overall is looking good to me, not sure why those unit tests are failing?

Elbehery · 2025-07-31T13:42:37Z

Code overall is looking good to me, not sure why those unit tests are failing?

i tried to fix them but seems unrelated to my changes 🤔

ashwinb · 2025-07-31T16:13:39Z

I forgot why I had added a separate initialize() method. There was a very important reason but I guess I forgot to document. This needs to be really tested in a notebook environment also btw, while I try to remember ...

mattf

please remove all extraneous changes

docs/source/distributions/importing_as_library.md

llama_stack/core/library_client.py

mattf · 2025-08-11T00:49:48Z

I forgot why I had added a separate initialize() method. There was a very important reason but I guess I forgot to document. This needs to be really tested in a notebook environment also btw, while I try to remember ...

@ashwinb probably __init__ can't be async

ashwinb · 2025-08-13T14:25:40Z

I forgot why I had added a separate initialize() method. There was a very important reason but I guess I forgot to document. This needs to be really tested in a notebook environment also btw, while I try to remember ...

@ashwinb probably init can't be async

Yeah. But I didn't know about loop.run_until_complete at that time also :)

llama_stack/core/library_client.py

mattf

why did you remove skip_logger_removal from the sync client and add it to the async client?

Elbehery · 2025-08-17T19:50:26Z

why did you remove skip_logger_removal from the sync client and add it to the async client?

thanks for your review 🙏🏽

so i received this review #2979 (comment), which actually makes sense imo :)

please let me know if you have other thoughts

Elbehery · 2025-08-18T22:37:44Z

Hi @ashwinb

Any more required changes here ?

If not, I would appreciate it very much if we merge this PR.

I have already 4 open PRs, and I can not pick up more work till I merge them :)

thanks in advance :)

Elbehery · 2025-08-19T02:43:23Z

rebased 👍🏽

llama_stack/core/library_client.py

mattf

downgrading my review to a comment. looks like @ashwinb has this handled.

Elbehery · 2025-08-20T21:33:25Z

rebased 👍🏽

cc @ashwinb

llama_stack/core/library_client.py

tests/integration/fixtures/common.py

tests/integration/non_ci/responses/fixtures/fixtures.py

Currently client.initialize() had to be invoked by user. To improve dev experience and to avoid runtime errors, this PR init LlamaStackAsLibrary implicitly upon using the client. It prevents also multiple init of the same client, while maintaining backward ccompatibility. Signed-off-by: Mustafa Elbehery <[email protected]>

…r removing initialize method The recent refactor (3778a4c) introduced automatic initialization for LlamaStackAsLibraryClient but the unit tests were expecting manual initalization and _is_initialized. This caused test failure. Changes: - Update test assertions to check route_impls is not None instead of _is_initialized - Add proper mocking in tests to avoid external provider dependencies - Maintain test coverage for automatic initialization behavior - Ensure backward compatibility testing for deprecated initialize() method Signed-off-by: Mustafa Elbehery <[email protected]>

…#2979) # What does this PR do?  This PR removes `init()` from `LlamaStackAsLibrary` Currently client.initialize() had to be invoked by user. To improve dev experience and to avoid runtime errors, this PR init LlamaStackAsLibrary implicitly upon using the client. It prevents also multiple init of the same client, while maintaining backward ccompatibility. This PR does the following - Automatic Initialization: Constructor calls initialize_impl() automatically. - Client is fully initialized after __init__ completes. - Prevents consecutive initialization after the client has been successfully initialized. - initialize() method still exists but is now a no-op.   fixes llamastack#2946 --------- Signed-off-by: Mustafa Elbehery <[email protected]>

Elbehery requested review from ashwinb, yanxi0830, hardikjshah, raghotham, ehhuang, terrytangyuan, leseb, bbrowning, reluctantfuturist and mattf as code owners July 31, 2025 12:20

meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 31, 2025

mattf requested changes Jul 31, 2025

View reviewed changes

Elbehery force-pushed the 20250731_remove_init_LlamaStackAsLibrary branch from 3eec93f to ea66d34 Compare August 1, 2025 10:05

ashwinb reviewed Aug 1, 2025

View reviewed changes

llama_stack/core/library_client.py Show resolved Hide resolved

ashwinb requested changes Aug 1, 2025

View reviewed changes

llama_stack/core/library_client.py Outdated Show resolved Hide resolved

Elbehery force-pushed the 20250731_remove_init_LlamaStackAsLibrary branch from ea66d34 to 74817a7 Compare August 13, 2025 16:52

Elbehery requested a review from slekkala1 as a code owner August 13, 2025 16:52

ashwinb requested changes Aug 13, 2025

View reviewed changes

llama_stack/core/library_client.py Outdated Show resolved Hide resolved

llama_stack/core/library_client.py Outdated Show resolved Hide resolved

llama_stack/core/library_client.py Outdated Show resolved Hide resolved

llama_stack/core/library_client.py Outdated Show resolved Hide resolved

Elbehery force-pushed the 20250731_remove_init_LlamaStackAsLibrary branch 6 times, most recently from 3334941 to 692843f Compare August 14, 2025 12:52

Elbehery force-pushed the 20250731_remove_init_LlamaStackAsLibrary branch from 269c854 to af21f69 Compare August 17, 2025 15:25

mattf reviewed Aug 17, 2025

View reviewed changes

Elbehery force-pushed the 20250731_remove_init_LlamaStackAsLibrary branch 4 times, most recently from 0ec41e9 to 78f7be7 Compare August 18, 2025 22:32

Elbehery force-pushed the 20250731_remove_init_LlamaStackAsLibrary branch from 78f7be7 to a39ca14 Compare August 19, 2025 02:42

derekhiggins reviewed Aug 19, 2025

View reviewed changes

llama_stack/core/library_client.py Show resolved Hide resolved

Elbehery force-pushed the 20250731_remove_init_LlamaStackAsLibrary branch 4 times, most recently from 07c04cf to a1c4f17 Compare August 20, 2025 12:43

mattf reviewed Aug 20, 2025

View reviewed changes

mattf self-requested a review August 20, 2025 15:39

Elbehery force-pushed the 20250731_remove_init_LlamaStackAsLibrary branch 2 times, most recently from 681182d to a8436ee Compare August 20, 2025 21:16

Elbehery force-pushed the 20250731_remove_init_LlamaStackAsLibrary branch from a8436ee to 3e3a380 Compare August 20, 2025 22:06

ashwinb requested changes Aug 20, 2025

View reviewed changes

llama_stack/core/library_client.py Outdated Show resolved Hide resolved

tests/integration/fixtures/common.py Show resolved Hide resolved

tests/integration/non_ci/responses/fixtures/fixtures.py Outdated Show resolved Hide resolved

Elbehery force-pushed the 20250731_remove_init_LlamaStackAsLibrary branch from 3e3a380 to 5bc261f Compare August 21, 2025 18:29

Elbehery added 2 commits August 22, 2025 00:51

Elbehery force-pushed the 20250731_remove_init_LlamaStackAsLibrary branch from 5bc261f to 2ebdfce Compare August 21, 2025 22:51

ashwinb approved these changes Aug 21, 2025

View reviewed changes

ashwinb merged commit 1790fc0 into llamastack:main Aug 21, 2025
22 checks passed

Elbehery deleted the 20250731_remove_init_LlamaStackAsLibrary branch August 21, 2025 23:03

feat: Remove initialize() Method from LlamaStackAsLibrary #2979

feat: Remove initialize() Method from LlamaStackAsLibrary #2979

Conversation

Elbehery commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Uh oh!

Elbehery commented Jul 31, 2025

Uh oh!

nathan-weinberg commented Jul 31, 2025

Uh oh!

Elbehery commented Jul 31, 2025

Uh oh!

ashwinb commented Jul 31, 2025

Uh oh!

mattf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattf commented Aug 11, 2025

Uh oh!

ashwinb commented Aug 13, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattf left a comment

Choose a reason for hiding this comment

Uh oh!

Elbehery commented Aug 17, 2025

Uh oh!

Elbehery commented Aug 18, 2025

Uh oh!

Elbehery commented Aug 19, 2025

Uh oh!

Uh oh!

mattf left a comment

Choose a reason for hiding this comment

Uh oh!

Elbehery commented Aug 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Elbehery commented Jul 31, 2025 •

edited

Loading