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

create a github action for triggering client-sdk tests on new pull-request #850

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sixianyi0721
Copy link
Contributor

@sixianyi0721 sixianyi0721 commented Jan 23, 2025

What does this PR do?

Create a new github action that runs integration tests on fireworks and together distro upon new PR

Key features:

  1. Run inference client-sdk tests on fireworks and together distro. Load distro as a library
  2. Pull changes from latest github repo (llama-models) and (llama-stack-client-python)
  3. output a test summary

Next steps:

  • Expand the ci test action to (llama-models) and (llama-stack-client-python) repo to make sure the changes there does not break the imports in llama-stack

Test Plan

See the job run triggered by this PR

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jan 23, 2025
@sixianyi0721 sixianyi0721 force-pushed the testci branch 20 times, most recently from 9a84b07 to 7a59e27 Compare January 23, 2025 09:57
@sixianyi0721 sixianyi0721 changed the title citest create a github action for triggering client-sdk tests on new pull-request Jan 23, 2025
@sixianyi0721 sixianyi0721 marked this pull request as ready for review January 23, 2025 10:02
Copy link
Contributor

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

This would be super useful. Thanks!

name: auto-tests

on:
pull_request:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should automatically run this -- not yet. Can you make sure you keep this action available so people can manually invoke it on a commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes we also allow manual trigger (line 5)

I don't think we should automatically run this -- not yet.

what is your consideration here?

test-llama-stack-as-library:
runs-on: ubuntu-latest
env:
TOGETHER_API_KEY: ${{ secrets.TOGETHER_API_KEY }}
Copy link
Contributor

Choose a reason for hiding this comment

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

who can access these secrets? can any public developer invoke the workflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

only us can access the secrets but any public developer can invoke the flow

@sixianyi0721 sixianyi0721 force-pushed the testci branch 7 times, most recently from ea89cb0 to 294b58c Compare January 23, 2025 22:50
provider: [fireworks, together]

# Dumb approach to restrict the users who trigger the gh actions
if: contains('["ashwinb", "yanxi0830", "hardikjshah", "dltn", "raghotham", "dineshyv", "vladimirivic", "sixianyi0721"]', github.actor)
Copy link
Contributor Author

@sixianyi0721 sixianyi0721 Jan 23, 2025

Choose a reason for hiding this comment

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

Updated such that only those in CODEOWNER files can trigger the gh action. Will follow up with a smarter approach

@sixianyi0721 sixianyi0721 force-pushed the testci branch 4 times, most recently from 705186d to 94b592d Compare January 24, 2025 01:15
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.

4 participants