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

[FEA]: Replace multiprocessing.Queue usage in python/morpheus/morpheus/utils/logger.py #2110

Closed
2 tasks done
dagardner-nv opened this issue Jan 15, 2025 · 1 comment · Fixed by #2112
Closed
2 tasks done
Assignees
Labels
feature request New feature or request

Comments

@dagardner-nv
Copy link
Contributor

Is this a new feature, an improvement, or a change to existing functionality?

Improvement

How would you describe the priority of this feature request

High

Please provide a clear description of problem this feature solves

Currently this class is raising a NotImplementedError on some platforms specifically on Arm64. This is a known issue in cpython (python/cpython#101094) and doesn't appear to be getting fixed anytime soon.

Describe your ideal solution

Use our own FiberQueue class instead.

Additional context

The test tests/morpheus/utils/test_logger.py is currently being skipped on ARM and should be re-enabled once this issue is resolved.

Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I have searched the open feature requests and have found no duplicates for this feature request
@dagardner-nv dagardner-nv added the feature request New feature or request label Jan 15, 2025
dagardner-nv added a commit to dagardner-nv/Morpheus that referenced this issue Jan 15, 2025
@dagardner-nv dagardner-nv self-assigned this Jan 16, 2025
@dagardner-nv dagardner-nv moved this from Todo to In Progress in Morpheus Boards Jan 16, 2025
@morpheus-bot-test morpheus-bot-test bot moved this from In Progress to Review - Ready for Review in Morpheus Boards Jan 16, 2025
@dagardner-nv
Copy link
Contributor Author

Turns out it was only a single unittest which was dependent upon the multiprocessing.Queue.qsize, and was easily worked-around by using the multiprocessing.Queue.empty method instead.

I did investigate using the FiberQueue class instead, however neither that class or the boost::fibers::buffered_channel class it encapsulates contains an empty or a size method. However it is possible that checking begin() == end() should be sufficient for implementing an empty method if we wanted to do so in the future.

@rapids-bot rapids-bot bot closed this as completed in cf8a9df Jan 17, 2025
@github-project-automation github-project-automation bot moved this from Review - Ready for Review to Done in Morpheus Boards Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant