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

Loosen the dependency version restriction (#499) #509

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

chishui
Copy link
Contributor

@chishui chishui commented Apr 7, 2024

Description

Most of the dependency versions in setup.py are outdated. The PR is to loosen the dependency version restriction so that OSB can be run in the same env with other dependency which requires different dependency version e.g. Jupyter notebook.

Issues Resolved

#499

Testing

  • New functionality includes testing

  • make test

  • opensearch-benchmark execute-test ...

  • pip install notebook, then run jupyter notebook, then run opensearch-benchmark execute-test ... again.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

setup.py Outdated
"twine>=1.15.0",
"wheel>=0.38.4",
"github3.py>=1.3.0",
"pylint>=2.6.0",
Copy link
Collaborator

@IanHoang IanHoang Apr 8, 2024

Choose a reason for hiding this comment

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

Try setting pylint back to == or restrict it to be under 3.X.X as it's currently downloading version 3.1.0 and there's a CI error that seems to be related to Pylint 3.0+ versions

Collecting pylint>=2.6.0 (from opensearch-benchmark==1.5.0)
  Downloading pylint-3.1.0-py3-none-any.whl.metadata (12 kB)

Error

ImportError: cannot import name 'IAstroidChecker' from 'pylint.interfaces' (/home/runner/work/opensearch-benchmark/opensearch-benchmark/.venv/lib/python3.8/site-packages/pylint/interfaces.py)

Copy link
Collaborator

@IanHoang IanHoang left a comment

Choose a reason for hiding this comment

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

proposed a change above for how to resolve CI errors.

@chishui chishui force-pushed the main branch 2 times, most recently from 77aab51 to 589a4f8 Compare April 9, 2024 02:56
@chishui
Copy link
Contributor Author

chishui commented Apr 9, 2024

I only kept the changes in install_requires section and reverted tests_require and develop_require as they caused build failing, now all tests passed.

Copy link
Collaborator

@IanHoang IanHoang left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for this!

@IanHoang IanHoang merged commit e056052 into opensearch-project:main Apr 9, 2024
8 checks passed
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.

2 participants