Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: update integration test workflow #1856

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mattf
Copy link
Contributor

@mattf mattf commented Apr 1, 2025

workflow -
0. Checkout

  1. Install uv
  2. Install Ollama
  3. Pull Ollama image
  4. Start Ollama in background
  5. Set Up Environment and Install Dependencies
  6. Wait for Ollama to start
  7. Start Llama Stack server in background
  8. Wait for Llama Stack server to be ready
  9. Run Integration Tests

changes -
(4) starts the loading of the ollama model, it does not start ollama. the model will be loaded when used. this step is removed.
(6) is handled in (2). this step is removed.
(2) is renamed to reflect it's dual purpose.

workflow -
 0. Checkout
 1. Install uv
 2. Install Ollama
 3. Pull Ollama image
 4. Start Ollama in background
 5. Set Up Environment and Install Dependencies
 6. Wait for Ollama to start
 7. Start Llama Stack server in background
 8. Wait for Llama Stack server to be ready
 9. Run Integration Tests

changes -
 (4) starts the loading of the ollama model, it does not start ollama. the model will be loaded when used. this step is removed.
 (6) is handled in (2). this step is removed.
 (2) is renamed to reflect it's dual purpose.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Apr 1, 2025
@mattf
Copy link
Contributor Author

mattf commented Apr 1, 2025

@leseb ptal

@mattf mattf changed the title update integration test workflow refactor: update integration test workflow Apr 1, 2025
@mattf
Copy link
Contributor Author

mattf commented Apr 1, 2025

the integration tests will pass once #1854 is merged

Copy link
Collaborator

@leseb leseb left a comment

Choose a reason for hiding this comment

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

Do you mind adding this in #1854? So we can validate both at the same time? Thanks!

@mattf
Copy link
Contributor Author

mattf commented Apr 7, 2025

Do you mind adding this in #1854? So we can validate both at the same time? Thanks!

my preference is small, contained changes.

this request can be independently rejected if it's not desired. it is not critical for proper functioning.

the other change is important for properly working functionality.

i can merge them if you want.

@leseb
Copy link
Collaborator

leseb commented Apr 7, 2025

Do you mind adding this in #1854? So we can validate both at the same time? Thanks!

my preference is small, contained changes.

Small contained changes can be achieved by commits, not always PRs.

this request can be independently rejected if it's not desired. it is not critical for proper functioning.

the other change is important for properly working functionality.

i can merge them if you want.

Given that you remove "ollama run" from the CI I was under the impression that this could help validate #1854 behavior. Did I miss something?

@mattf
Copy link
Contributor Author

mattf commented Apr 7, 2025

this request can be independently rejected if it's not desired. it is not critical for proper functioning.
the other change is important for properly working functionality.
i can merge them if you want.

Given that you remove "ollama run" from the CI I was under the impression that this could help validate #1854 behavior. Did I miss something?

the existing workflow (without this PR) will also validate #1854, since a run does a pull.

@leseb
Copy link
Collaborator

leseb commented Apr 7, 2025

this request can be independently rejected if it's not desired. it is not critical for proper functioning.
the other change is important for properly working functionality.
i can merge them if you want.

Given that you remove "ollama run" from the CI I was under the impression that this could help validate #1854 behavior. Did I miss something?

the existing workflow (without this PR) will also validate #1854, since a run does a pull.

Yes, but isn't the goal to just do a "pull" and let ollama do the "run" when a request comes in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants