-
Notifications
You must be signed in to change notification settings - Fork 909
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
fix: test_get_incomplete_run #1983
Conversation
# Description Please add an informative description that covers that changes made by the pull request and link all relevant issues. # All Promptflow Contribution checklist: - [ ] **The pull request does not introduce [breaking changes].** - [ ] **CHANGELOG is updated for new features, bug fixes or other significant changes.** - [ ] **I have read the [contribution guidelines](../CONTRIBUTING.md).** - [ ] **Create an issue and link to the pull request to get dedicated review from promptflow team. Learn more: [suggested workflow](../CONTRIBUTING.md#suggested-workflow).** ## General Guidelines and Best Practices - [ ] Title of the pull request is clear and informative. - [ ] There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md). ### Testing Guidelines - [ ] Pull request includes test coverage for the included changes.
# Description Please add an informative description that covers that changes made by the pull request and link all relevant issues. # All Promptflow Contribution checklist: - [x] **The pull request does not introduce [breaking changes].** - [ ] **CHANGELOG is updated for new features, bug fixes or other significant changes.** - [ ] **I have read the [contribution guidelines](../CONTRIBUTING.md).** - [ ] **Create an issue and link to the pull request to get dedicated review from promptflow team. Learn more: [suggested workflow](../CONTRIBUTING.md#suggested-workflow).** ## General Guidelines and Best Practices - [x] Title of the pull request is clear and informative. - [ ] There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md). ### Testing Guidelines - [x] Pull request includes test coverage for the included changes.
Test Results 42 files 42 suites 9h 57m 6s ⏱️ For more details on these failures and errors, see this check. Results for commit ef57970. |
SDK CLI Global Config Test Result zhangxingzhi/fix-get-incomplete-run2 tests 2 ✅ 49s ⏱️ Results for commit ef57970. |
promptflow SDK CLI Azure E2E Test Result zhangxingzhi/fix-get-incomplete-run 4 files 4 suites 3m 20s ⏱️ Results for commit ef57970. |
SDK CLI Test Result zhangxingzhi/fix-get-incomplete-run 12 files 12 suites 39m 25s ⏱️ Results for commit ef57970. |
@@ -1295,7 +1295,9 @@ def test_get_incomplete_run(self, local_client, pf) -> None: | |||
) | |||
|
|||
# remove run dag | |||
shutil.rmtree(f"{temp_dir}/print_env_var") | |||
# TODO 2945229: Han to check whether this is expected - can't delete the whole directory given .promptflow | |||
# is occupied |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why .promptflow is occupied?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also curious with the same thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is temporary directory like /tmp/xxxx/print_env_var or C:\User\xxx\Temp\print_env_var.
But it is still weird why the folder is not released after pf run.
Hi, thank you for your interest in helping to improve the prompt flow experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. |
Hi, thank you for your contribution. Since there has not been recent engagement, we are going to close this out. Feel free to reopen if you'd like to continue working on these changes. Please be sure to remove the |
Description
This pull request includes a change in the
test_get_incomplete_run
method in thesrc/promptflow/tests/sdk_cli_test/e2etests/test_flow_run.py
file. The change replaces theshutil.rmtree
function, which was used to delete the entire directory, with theos.remove
function to only delete theflow.dag.yaml
file. This change is due to the.promptflow
directory being occupied, which prevents the deletion of the entire directory. A comment was added to remind to check whether this behavior is expected.https://github.com/microsoft/promptflow/actions/runs/7795616592/job/21258881713
All Promptflow Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines