Skip to content

Conversation

@Adolfi
Copy link

@Adolfi Adolfi commented Aug 15, 2025

  • Added missing cs test for StartModelAsync_Succeeds_WhenModelIsInCatalogAndCache to verify that model can be started from local cache.
  • Added missing cs test for StartModelAsync_Succeeds_WhenModelIsDownloaded to verify that model can be downloaded and started if the model is not found in the local cache.
  • Changed return type for StartModelAsync to ModelInfo. Previously returns a new FoundryLocalManager which seems very strange? DownloadModelAsync, GetModelInfoAsync, UpgradeModelAsync and LoadModelAsync all returns ModelInfo so it seems reasonable that StartModelAsync would also do such. I realize that I'm possibly breaking backward compability buth rather now in preview than later.
  • Removed static modifier for the method StartModelAsync. It is not statically accessed anywhere and compiles just fine without it. Is there a reason why this particular method is static when no other method in the FoundrLocalManager is?
  • Updated structure in StartModelAsync for consistency and testability:
    -- Previously created a new FoundryLocalManager (probably since it was static declared) which makes is inconsistent and hard to test/mock.
    -- Also it wrapped its functionality in a try/catch to be able to dispose the newly created FoundryLocalManager. This is not done anywhere else in the FoundryLocalManager methods and could be very confusing. Leave the disposing to the consumer of the FoundryLocalManager.

Adolfi added 2 commits August 15, 2025 09:42
…e to verify that model can be started from local cache.

- Added test for StartModelAsync_Succeeds_WhenModelIsDownloaded to verify that model can be started from empty cache which downloads model.
…as alla other methods in FoundryLocalManager
@Adolfi
Copy link
Author

Adolfi commented Aug 15, 2025

@microsoft-github-policy-service agree

Removed FoundryLocalManager constructor according to latest main.
Updated tests to work with new main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant