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

vdk-core: new version check built-in plugin false positive fix #816

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

ivakoleva
Copy link
Contributor

A bug detected when using custom new-version-check-plugin
configurations:

config_builder.set_value(key="PACKAGE_INDEX",value="https://some.repo")
config_builder.set_value(key="PACKAGE_NAME",
value="some-package-upgraded-to-latest-locally")
config_builder.set_value("VERSION_CHECK_PLUGINS", False)

That results in false-positive yet package-empty
upgrade message displayed:

************************************************************************
New version for   is available.
Please update to latest version by using:
 pip install --upgrade-strategy eager -U   --extra-index-url
 https://some.repo
************************************************************************

Changed _check_version method to _print_new_version_message,
matching the purpose described in its pydocs. Added handling of
use-case when no packages for upgrade detected.

Testing Done: added a test_no_new_version_check to cover that
use-case, the test detects the issue before the changes
and succeeds after applying the changes made.

Signed-off-by: ikoleva [email protected]

A bug detected when using custom new-version-check-plugin
configurations:
```
config_builder.set_value(key="PACKAGE_INDEX",value="https://some.repo")
config_builder.set_value(key="PACKAGE_NAME",
value="some-package-upgraded-to-latest-locally")
config_builder.set_value("VERSION_CHECK_PLUGINS", False)
```
That results in false-positive yet package-empty
upgrade message displayed:
```
************************************************************************
New version for   is available.
Please update to latest version by using:
 pip install --upgrade-strategy eager -U   --extra-index-url
 https://some.repo
************************************************************************
```

Changed `_check_version` method to `_print_new_version_message`,
matching the purpose described in its pydocs. Added handling of
use-case when no packages for upgrade detected.

Testing Done: added a `test_no_new_version_check` to cover that
use-case, the test detects the issue before the changes
and succeeds after applying the changes made.

Signed-off-by: ikoleva <[email protected]>
@ivakoleva ivakoleva enabled auto-merge (squash) April 26, 2022 13:04
Copy link
Contributor

@YanaZhivkova YanaZhivkova left a comment

Choose a reason for hiding this comment

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

LGTM

@ivakoleva ivakoleva merged commit fa2708f into main Apr 26, 2022
@ivakoleva ivakoleva deleted the person/ikoleva/new-version-check-fix branch April 26, 2022 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants