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

Hint user to upgrade every 7 days for HINT_ACTIVITY_NAME #1407

Merged
merged 42 commits into from
Dec 21, 2023

Conversation

YingChen1996
Copy link
Contributor

@YingChen1996 YingChen1996 commented Dec 6, 2023

This pull request includes several changes to the promptflow package, adding new test methods, constants, and modules related to version hinting and checking. The most important changes include adding a new test method with parameterized values, adding new constants to the AvailableIDE class, and adding a new module for version hinting and checking.

Main changes:

  • src/promptflow/tests/sdk_cli_test/unittests/test_utils.py: Added a new test method test_concurrent_hint_for_update with parameterized values for concurrent_count.
  • src/promptflow/promptflow/_constants.py: Added new constants related to version hinting and checking in the AvailableIDE class.
  • src/promptflow/promptflow/_utils/version_hint_utils.py: Added a new module version_hint_utils.py with functions for version hinting and checking.
    image

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.
  • Create an issue and link to the pull request to get dedicated review from promptflow team. Learn more: 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.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

Copy link

github-actions bot commented Dec 6, 2023

SDK CLI Global Config Test Result chenyin/hint_upgrade

2 tests   2 ✔️  40s ⏱️
1 suites  0 💤
1 files    0

Results for commit c5c1035.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 6, 2023

Executor Unit Test Result chenyin/hint_upgrade

598 tests   595 ✔️  45s ⏱️
    1 suites      3 💤
    1 files        0

Results for commit c5c1035.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 6, 2023

promptflow SDK CLI Azure E2E Test Result chenyin/hint_upgrade

    4 files      4 suites   2m 59s ⏱️
116 tests   98 ✔️ 18 💤 0
464 runs  392 ✔️ 72 💤 0

Results for commit c5c1035.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 6, 2023

Executor E2E Test Result chenyin/hint_upgrade

161 tests   159 ✔️  2m 16s ⏱️
    1 suites      2 💤
    1 files        0

Results for commit c5c1035.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 6, 2023

SDK CLI Test Result chenyin/hint_upgrade

     12 files       12 suites   10m 44s ⏱️
   337 tests    331 ✔️   6 💤 0
1 348 runs  1 324 ✔️ 24 💤 0

Results for commit c5c1035.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 6, 2023

SDK PFS E2E Test Result chenyin/hint_upgrade

  2 files    2 suites   1m 29s ⏱️
16 tests 16 ✔️ 0 💤 0
32 runs  32 ✔️ 0 💤 0

Results for commit a3f084a.

♻️ This comment has been updated with latest results.

zhengfeiwang
zhengfeiwang previously approved these changes Dec 21, 2023
wangchao1230
wangchao1230 previously approved these changes Dec 21, 2023
@YingChen1996 YingChen1996 merged commit 855ac17 into main Dec 21, 2023
42 checks passed
@YingChen1996 YingChen1996 deleted the chenyin/hint_upgrade branch December 21, 2023 11:45
hint_for_update()
with ThreadPoolExecutor(max_workers=concurrent_count) as pool:
pool.submit(check_latest_version)

Copy link
Contributor

Choose a reason for hiding this comment

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

Will there be cases check_latest_version not finished when running the following line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we won't check since Clement suggests may not break user because of get pypi.

mock_datetime.datetime.strptime.return_value = datetime.datetime.now() - datetime.timedelta(days=8)
mock_datetime.timedelta.return_value = datetime.timedelta(days=7)
hint_for_update()
with ThreadPoolExecutor(max_workers=concurrent_count) as pool:
Copy link
Contributor

Choose a reason for hiding this comment

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

How does max_workers impact check_latest_version's behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this is added when hint_for_update and check_latest_version need to be run in one thread pool and I need check the file is well writen in max_workers. Will update this test in later pr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
promptflow sdk prompt flow SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants