Project

General

Profile

Actions

Task #4555

closed

SegmentFetcher: eliminate selector usage

Added by Junxiao Shi about 6 years ago. Updated about 3 years ago.

Status:
Closed
Priority:
High
Assignee:
Category:
Utils
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
1.50 h

Description

SegmentFetcher currently uses ChildSelector. This usage shall be eliminated as part of Packet03Transition.


Related issues 1 (0 open1 closed)

Blocked by ndn-cxx - Task #4464: Make SegmentFetcher pure Signal-basedClosedEric Newberry

Actions
Actions #1

Updated by Junxiao Shi almost 6 years ago

  • Assignee set to Eric Newberry

Change 4561 patchset 6 changes interest.setChildSelector(1) to interest.setCanBePrefix(false). Consequently, the caller of SegmentFetcher::fetch is expected to pass an exact name including version number and segment number components, and SegmentFetcher would not attempt to prefer the latest version.
While I support adopting this new semantics, it must be performed in a separate commit under this issue, not combined with another change. A 5-day breaking change notice is also required.

Actions #2

Updated by Eric Newberry almost 6 years ago

  • Start date deleted (03/24/2018)

Junxiao Shi wrote:

Change 4561 patchset 6 changes interest.setChildSelector(1) to interest.setCanBePrefix(false). Consequently, the caller of SegmentFetcher::fetch is expected to pass an exact name including version number and segment number components, and SegmentFetcher would not attempt to prefer the latest version.
While I support adopting this new semantics, it must be performed in a separate commit under this issue, not combined with another change. A 5-day breaking change notice is also required.

I don't want to delay everything by five days (also, where does it say we have to delay by 5 days for breaking changes?), so I'll do this last.

Also, I don't think the end user is required to specify a name that already includes the sequence number component to the fetch function. This is added by the retrieval logic inside SegmentFetcher.

Actions #3

Updated by Junxiao Shi almost 6 years ago

I don't want to delay everything by five days

As soon as the initial code is ready, you can send email notice to ndn-lib and nfd-dev.

where does it say we have to delay by 5 days for breaking changes?

Felix Rabe demanded this a few years ago.
0.6.1 cycle was an exception because a general breaking changes notice about certificate v2 was sent.

I don't think the end user is required to specify a name that already includes the sequence number component to the fetch function. This is added by the retrieval logic inside SegmentFetcher.

That's not what 4651,6 does. Either way is fine.

Actions #4

Updated by Eric Newberry almost 6 years ago

Junxiao Shi wrote:

I don't think the end user is required to specify a name that already includes the sequence number component to the fetch function. This is added by the retrieval logic inside SegmentFetcher.

That's not what 4651,6 does. Either way is fine.

No, 4651,6 definitely adds the segment component to the Interest.

Actions #5

Updated by Eric Newberry almost 6 years ago

Eric Newberry wrote:

Junxiao Shi wrote:

I don't think the end user is required to specify a name that already includes the sequence number component to the fetch function. This is added by the retrieval logic inside SegmentFetcher.

That's not what 4651,6 does. Either way is fine.

No, 4651,6 definitely adds the segment component to the Interest.

There's actually a typo (the last component of the base Interest name will be incorrectly removed) but the above statement still holds.

Actions #6

Updated by Davide Pesavento almost 6 years ago

  • Priority changed from Normal to High
Actions #7

Updated by Davide Pesavento almost 6 years ago

  • Blocked by Task #4464: Make SegmentFetcher pure Signal-based added
Actions #8

Updated by Eric Newberry almost 6 years ago

  • Status changed from New to In Progress
Actions #9

Updated by Eric Newberry almost 6 years ago

Junxiao Shi wrote:

Change 4561 patchset 6 changes interest.setChildSelector(1) to interest.setCanBePrefix(false). Consequently, the caller of SegmentFetcher::fetch is expected to pass an exact name including version number and segment number components, and SegmentFetcher would not attempt to prefer the latest version.
While I support adopting this new semantics, it must be performed in a separate commit under this issue, not combined with another change. A 5-day breaking change notice is also required.

But don't we want to prefer the latest version? If I'm understanding the above correctly, these semantics would not prefer the latest version.

Actions #10

Updated by Eric Newberry almost 6 years ago

Eric Newberry wrote:

Junxiao Shi wrote:

Change 4561 patchset 6 changes interest.setChildSelector(1) to interest.setCanBePrefix(false). Consequently, the caller of SegmentFetcher::fetch is expected to pass an exact name including version number and segment number components, and SegmentFetcher would not attempt to prefer the latest version.
While I support adopting this new semantics, it must be performed in a separate commit under this issue, not combined with another change. A 5-day breaking change notice is also required.

But don't we want to prefer the latest version? If I'm understanding the above correctly, these semantics would not prefer the latest version.

Or would MustBeFresh=true handle this?

Actions #11

Updated by Eric Newberry almost 6 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 0 to 100

Pushed a change that switches SegmentFetcher over to using CanBePrefix instead of ChildSelector so we can start discussing the best approach to use.

Actions #12

Updated by Junxiao Shi almost 6 years ago

But don't we want to prefer the latest version? If I'm understanding the above correctly, these semantics would not prefer the latest version.
Or would MustBeFresh=true handle this?

There isn't a simple way to ensure the latest version (#4396), but CanBePrefix=true MustBeFresh=true prefers the latest version, and thus preserves the previous semantics.

Actions #13

Updated by Eric Newberry almost 6 years ago

Junxiao Shi wrote:

But don't we want to prefer the latest version? If I'm understanding the above correctly, these semantics would not prefer the latest version.
Or would MustBeFresh=true handle this?

There isn't a simple way to ensure the latest version (#4396), but CanBePrefix=true MustBeFresh=true prefers the latest version, and thus preserves the previous semantics.

Ok, thanks for the clarification.

Actions #14

Updated by Eric Newberry almost 6 years ago

  • Status changed from Code review to Closed
Actions #15

Updated by Davide Pesavento about 3 years ago

  • Tracker changed from Feature to Task
Actions

Also available in: Atom PDF