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

Refactoring #cellDiscarded message creation and reception #105

Merged
merged 2 commits into from
Apr 5, 2020

Conversation

darth-cheney
Copy link
Contributor

This PR restructures the way cellDiscarded (now cellsDiscarded) messages are composed, received, and ultimately handled on the Component side. This is in relation to #104

Motivation and Context

See Issue #104 for details. We were sending (potentially) too many messages for discarding cells per rendering cycle

Approach

Instead of composing a single message per Cell being discarded, we now send over an array of all ids of cells to be discarded at the same time.

The CellHandler has also been updated to deal with this change.

We have changed all references to cellDiscarded and renamed them cellsDiscarded

How Has This Been Tested?

All existing tests are passing as normal

Screenshots or console output (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@codecov
Copy link

codecov bot commented Mar 20, 2020

Codecov Report

Merging #105 into dev will decrease coverage by 0%.
The diff coverage is 100%.

Impacted file tree graph

@@         Coverage Diff         @@
##            dev   #105   +/-   ##
===================================
- Coverage    62%    62%   -1%     
===================================
  Files        86     86           
  Lines      9090   9095    +5     
===================================
- Hits       5693   5659   -34     
- Misses     3397   3436   +39     
Impacted Files Coverage Δ
object_database/web/cells/Messenger.py 89% <100%> (-2%) ⬇️
object_database/web/cells/cells.py 54% <100%> (+<1%) ⬆️
object_database/service_manager/Task.py 28% <0%> (-23%) ⬇️
object_database/database_connection.py 90% <0%> (+<1%) ⬆️
object_database/tcp_server.py 92% <0%> (+<1%) ⬆️
object_database/service_manager/ServiceBase.py 94% <0%> (+30%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4910266...7ca547e. Read the comment docs.

@darth-cheney
Copy link
Contributor Author

@braxtonmckee LMK if you are good to merge this. Would be useful to have for some ongoing work I'm doing. Should save a lot of bits going over the wire.

@braxtonmckee braxtonmckee merged commit c71a16b into dev Apr 5, 2020
@braxtonmckee braxtonmckee deleted the eric-issue-104 branch April 5, 2020 13:56
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