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

Patch/add azure to workflows #1537

Merged
merged 6 commits into from
Oct 30, 2024
Merged

Conversation

emrgnt-cmplxty
Copy link
Contributor

@emrgnt-cmplxty emrgnt-cmplxty commented Oct 30, 2024

Important

Integrates Azure configurations into workflows, updates orchestration logic, and modifies tests to support Azure settings.

  • Azure Configuration:
    • Added full_azure.toml and r2r_azure.toml for Azure-specific settings.
    • Updated start-r2r-full/action.yml and start-r2r-light/action.yml to use Azure configurations.
  • Workflow Changes:
    • Added Azure environment variables to r2r-full-integration-deep-dive-tests.yml, r2r-js-sdk-integration-tests.yml, and r2r-light-py-integration-tests.yml.
    • Removed dev and dev-minor branches from r2r-full-py-integration-tests.yml and r2r-light-py-integration-tests.yml.
    • Deleted Mac and Windows integration test workflows.
  • Orchestration and API:
    • Added run_with_orchestration option in ingestion_router.py and kg_router.py to toggle orchestration.
    • Updated simple/kg_workflow.py to handle orchestration logic.
  • Testing:
    • Renamed runner_sdk.py to runner_sdk_basic.py.
    • Added local_harness.py for local test execution.
    • Updated test scripts to use runner_sdk_basic.py.

This description was created by Ellipsis for d75dd97. It will automatically update as commits are pushed.

Copy link

vercel bot commented Oct 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
yc_demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 30, 2024 8:49pm
yc-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 30, 2024 8:49pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
recommendation_platform ⬜️ Ignored (Inspect) Oct 30, 2024 8:49pm

@emrgnt-cmplxty emrgnt-cmplxty marked this pull request as ready for review October 30, 2024 18:58
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 954c59b in 2 minutes and 10 seconds

More details
  • Looked at 2269 lines of code in 28 files
  • Skipped 0 files when reviewing.
  • Skipped posting 9 drafted comments based on config settings.
1. .github/actions/run-sdk-auth-tests/action.yml:10
  • Draft comment:
    The test runner has been changed from runner_sdk.py to runner_sdk_basic.py. Ensure that runner_sdk_basic.py contains all necessary tests and configurations that were present in runner_sdk.py. This change is also present in other workflow files.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR changes the test runner to use runner_sdk_basic.py instead of runner_sdk.py. This change is consistent across multiple files, indicating a shift in the testing strategy. However, the PR description does not provide context for this change, which could be important for understanding the intent and ensuring the correct tests are being run.
2. .github/actions/start-r2r-full/action.yml:15
  • Draft comment:
    The configuration has been changed from full to full_azure. Ensure that the full_azure configuration file is correctly set up for the intended tests. This change is likely intended to test Azure-specific configurations.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR modifies the ingestion command to use a different configuration file (full_azure instead of full). This change is likely intended to test Azure-specific configurations. However, the PR description does not provide context for this change, which could be important for understanding the intent and ensuring the correct configurations are being tested.
3. .github/actions/start-r2r-light/action.yml:10
  • Draft comment:
    The configuration for r2r serve has been changed to r2r_azure. Ensure that the r2r_azure configuration file is correctly set up for the intended tests. This change is likely intended to test Azure-specific configurations.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR changes the configuration for the r2r serve command to use r2r_azure. This change is likely intended to test Azure-specific configurations. However, the PR description does not provide context for this change, which could be important for understanding the intent and ensuring the correct configurations are being tested.
4. .github/workflows/r2r-full-py-integration-tests-graphrag.yml:6
  • Draft comment:
    Branches dev and dev-minor have been removed from the trigger list. Ensure that these branches are no longer needed for triggering this workflow.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR removes branches dev and dev-minor from the list of branches that trigger the workflow. This change might be intended to streamline the CI/CD process, but it should be confirmed that these branches are no longer needed for triggering tests.
5. .github/workflows/r2r-full-py-integration-tests.yml:6
  • Draft comment:
    Branches dev and dev-minor have been removed from the trigger list. Ensure that these branches are no longer needed for triggering this workflow.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR removes branches dev and dev-minor from the list of branches that trigger the workflow. This change might be intended to streamline the CI/CD process, but it should be confirmed that these branches are no longer needed for triggering tests.
6. .github/workflows/r2r-light-py-integration-tests-graphrag.yml:6
  • Draft comment:
    Branches dev and dev-minor have been removed from the trigger list. Ensure that these branches are no longer needed for triggering this workflow.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR removes branches dev and dev-minor from the list of branches that trigger the workflow. This change might be intended to streamline the CI/CD process, but it should be confirmed that these branches are no longer needed for triggering tests.
7. py/core/main/api/ingestion_router.py:413
  • Draft comment:
    The return type of ingest_chunks_app has been changed from a dictionary to a list containing a dictionary. Ensure that all client code is updated to handle the new response format.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR changes the return type of ingest_chunks_app from a dictionary to a list containing a dictionary. This change might affect how the response is handled by clients expecting a dictionary. Ensure that all client code is updated to handle the new response format.
8. py/core/main/api/kg_router.py:140
  • Draft comment:
    A new parameter run_with_orchestration has been added. Ensure that this parameter is correctly handled in all relevant parts of the codebase.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR adds a new parameter run_with_orchestration to the create_graph and enrich_graph methods. This parameter allows the user to choose whether to run with orchestration. Ensure that this parameter is correctly handled in all relevant parts of the codebase.
9. py/core/configs/full_azure.toml:1
  • Draft comment:
    full_azure.toml is a duplicate of r2r_azure.toml. Both files have nearly identical settings for Azure configurations. Consider consolidating them to avoid redundancy.

  • r2r_azure.toml

  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_Jn7FV128sHI2Kn97


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -0,0 +1,40 @@
# A config which overrides all instances of `openai` with `azure` in the `r2r.toml` config
Copy link
Contributor

Choose a reason for hiding this comment

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

The r2r_azure.toml file is a duplicate of the existing r2r.toml file, with the main difference being the provider (Azure vs OpenAI). Consider refactoring to avoid duplication.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 7b070da in 28 seconds

More details
  • Looked at 89 lines of code in 6 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_4mn8IPbSdR9RUMRV


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 7dac6f4 in 28 seconds

More details
  • Looked at 64 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_zNVvWUrQevuyJn6Y


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@emrgnt-cmplxty emrgnt-cmplxty merged commit 524b381 into dev-minor Oct 30, 2024
15 of 24 checks passed
emrgnt-cmplxty added a commit that referenced this pull request Oct 31, 2024
* improve ci/cd runtime (#1535)

* improve ci/cd runtime

* update prompt tests

* Support Python ^3.10 (#1534)

* Patch/add azure to workflows (#1537)

* improve ci/cd runtime

* update prompt tests

* add azure

* up

* up

* revert latest ingestion changes (#1539)

* revert latest ingestion changes

* revert latest ingestion changes

* Update pyproject.toml

---------

Co-authored-by: Nolan Tremelling <34580718+NolanTrem@users.noreply.github.com>
NolanTrem added a commit that referenced this pull request Oct 31, 2024
* improve ci/cd runtime (#1535)

* improve ci/cd runtime

* update prompt tests

* Support Python ^3.10 (#1534)

* Patch/add azure to workflows (#1537)

* improve ci/cd runtime

* update prompt tests

* add azure

* up

* up

* revert latest ingestion changes (#1539)

* revert latest ingestion changes

* revert latest ingestion changes

* Update pyproject.toml

* Update local_llm.toml (#1542)

* Update local_llm.toml

* Update pyproject.toml

* Update full_azure.toml

* expose user verification code

---------

Co-authored-by: Nolan Tremelling <34580718+NolanTrem@users.noreply.github.com>
@emrgnt-cmplxty emrgnt-cmplxty deleted the patch/add-azure-to-workflows branch December 5, 2024 00:48
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