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

Guard optional dependencies in try/except blocks #1382

Merged

Conversation

dagardner-nv
Copy link
Contributor

@dagardner-nv dagardner-nv commented Nov 22, 2023

Description

  • Add missing dependencies for requests, requests-cache and websockets to meta.yaml
  • Dependencies which are only utilized by a single stage are intentionally omitted from meta.yaml, even if they appear in cuda11.8_dev.yml
  • Current that list is: bs4, elasticsearch, feedparser and milvus
  • Move MILVUS_DATA_TYPE_MAP from morpheus/service/vdb/milvus_client.py to tests/test_milvus_vector_db_service.py which was the only place it was used.

Note: Since these dependencies all exist in cuda11.8_dev.yml, there is no effort made to update the tests to optionally skip their associated tests.

Closes #1380

By Submitting this PR I confirm:

  • I am familiar with the Contributing Guidelines.
  • When the PR is ready for review, new or existing tests cover these changes.
  • When the PR is ready for review, the documentation is up to date with these changes.

@dagardner-nv dagardner-nv self-assigned this Nov 22, 2023
@dagardner-nv dagardner-nv requested review from a team as code owners November 22, 2023 21:41
@dagardner-nv dagardner-nv marked this pull request as draft November 22, 2023 21:41
@dagardner-nv dagardner-nv added bug Something isn't working dependencies Pull requests that update a dependency file non-breaking Non-breaking change labels Nov 22, 2023
@dagardner-nv dagardner-nv changed the title Draft: Update conda build dependencies Update conda build dependencies Nov 22, 2023
@dagardner-nv dagardner-nv marked this pull request as ready for review November 22, 2023 22:31
Copy link
Contributor

@mdemoret-nv mdemoret-nv left a comment

Choose a reason for hiding this comment

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

Have some questions. We can add these dependencies but they arent used in many situations. Can we get by with an import guard instead to instruct the user to install these packages if this particular node type is used?

ci/conda/recipes/morpheus/meta.yaml Outdated Show resolved Hide resolved
ci/conda/recipes/morpheus/meta.yaml Outdated Show resolved Hide resolved
@dagardner-nv dagardner-nv changed the title Update conda build dependencies Guard optional dependencies in try/except blocks Nov 27, 2023
Copy link
Contributor

@mdemoret-nv mdemoret-nv left a comment

Choose a reason for hiding this comment

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

Looks better now. Will eventually need to re-address some of these in #1389

@mdemoret-nv
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit 119ec42 into nv-morpheus:branch-23.11 Nov 27, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Pull requests that update a dependency file non-breaking Non-breaking change
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[BUG]: Missing import feedparser required by RSSController
2 participants