Don't pre-convert all serverids to ServerTrackers #2119

New Issue

markberger · 2013-11-27T21:04:39Z

markberger commented

2013-11-27 21:04:39 +00:00

In immutable/upload.py we have the following code:

    def _get_next_allocation(self):
        """
        Return the next share allocation that we need to make.

        Specifically, I return a tuple (tracker, shares_to_ask), where
        tracker is a ServerTracker instance and shares_to_ask is a set of
        shares that we should store on that server. If there are no more
        allocations to make, I return None.
        """

        while self.trackers:
            tracker = self.trackers.pop(0)
            # TODO: don't pre-convert all serverids to ServerTrackers
            assert isinstance(tracker, ServerTracker)

As part of #1382 I am removing the TODO comment and creating a ticket for it instead.

In `immutable/upload.py` we have the following code: ``` def _get_next_allocation(self): """ Return the next share allocation that we need to make. Specifically, I return a tuple (tracker, shares_to_ask), where tracker is a ServerTracker instance and shares_to_ask is a set of shares that we should store on that server. If there are no more allocations to make, I return None. """ while self.trackers: tracker = self.trackers.pop(0) # TODO: don't pre-convert all serverids to ServerTrackers assert isinstance(tracker, ServerTracker) ``` As part of #1382 I am removing the TODO comment and creating a ticket for it instead.

markberger added the

labels 2013-11-27 21:04:39 +00:00

markberger added this to the undecided milestone 2013-11-27 21:04:39 +00:00

zooko commented

2013-11-29 05:38:28 +00:00

So, why not convert all serverids to ServerTrackers right away? Looking at the code for ServerTracker, it doesn't look like creating it has any problematic side-effects or uses up major resources, so unless there's something I didn't notice, I propose we just close this ticket as wontfix. Re-assigning to markberger to explain why this would be better.

By the way, good job cleaning up the TODOs out of the source code! ☺

So, why *not* convert all serverids to ServerTrackers right away? Looking at the code for ServerTracker, it doesn't look like creating it has any problematic side-effects or uses up major resources, so unless there's something I didn't notice, I propose we just close this ticket as `wontfix`. Re-assigning to markberger to explain why this would be better. By the way, good job cleaning up the TODOs out of the source code! ☺