Project

General

Profile

Actions

Feature #4207

closed

ndnpeek: set forwarding hint

Added by Junxiao Shi over 6 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Start date:
Due date:
% Done:

100%

Estimated time:
3.00 h

Description

In ndnpeek, introduce a new command line argument to set ForwardingHint on the Interest.
Delegations of the ForwardingHint shall be directly specified on the command line.
This replaces the --link-file command line argument.

Actions #1

Updated by Junxiao Shi over 6 years ago

  • Assignee set to Teng Liang

I'd let Teng kill off his own child (I mean, --link-file which he added in #3112).

Actions #2

Updated by Junxiao Shi over 6 years ago

  • Priority changed from Normal to Urgent

Increasing priority. ndn-tools build is broken by https://gerrit.named-data.net/4090 because makeLink function is using deprecated and now removed Link constructor overload. https://travis-ci.org/yoursunny/ndn-cxx-breaks/jobs/262098064

Actions #3

Updated by Davide Pesavento over 6 years ago

Junxiao, you should have fixed ndnpeek before removing the deprecated functions. It looks bad that we have a core tool broken for several days.

Actions #4

Updated by Junxiao Shi over 6 years ago

should have fixed ndnpeek before removing the deprecated functions

I did not know it would be broken. Although I triggered the build, I didn't look at the logs until the Change was merged.

It looks bad that we have a core tool broken for several days.

ndnpeek is by no means a "core tool". I rarely use it in my deployments.
repo-ng and ndns are broken for months.
ndn-cxx has issued a "breaking changes warning" in last release notes, and breaking changes are no longer subject to 5-day notice period.
ndn-cxx would even merge change that breaks NFD, which I'm unhappy about.

In either case, I won't work on this issue, and I'll let Teng handle it.

Actions #5

Updated by Davide Pesavento over 6 years ago

Quick fix for the build failure: https://gerrit.named-data.net/4120

Actions #6

Updated by Davide Pesavento over 6 years ago

Junxiao Shi wrote:

I did not know it would be broken. Although I triggered the build, I didn't look at the logs until the Change was merged.

That's not an excuse.

ndnpeek is by no means a "core tool". I rarely use it in my deployments.

Yes it is. Together with chunks and ping, it's one of the first tools that people use when they want to try things out. Luckily, the build failure affects only the unit tests, so probably most users are not impacted.

Actions #7

Updated by Alex Afanasyev over 6 years ago

We had multiple accidents like this even though we are always cautious, so let us be constructive.

Actions #8

Updated by Davide Pesavento over 6 years ago

My suggestion is to revert the commit that removed the deprecated functions from ndn-cxx.

Actions #9

Updated by Alex Afanasyev over 6 years ago

I would be against that this time. That was quite ugly API and I prefer we switch to the better (and now the only correct based on spec) API of forwarding hint.

Actions #10

Updated by Davide Pesavento over 6 years ago

  • Priority changed from Urgent to High

Downgrading priority since the build issue has been fixed for now.

Actions #11

Updated by Davide Pesavento over 6 years ago

Is Teng still working on this?

Actions #12

Updated by Teng Liang about 6 years ago

I've forgotten this issue. If someone else wants to work on it, just take it, or I'll start working on it within this week.

Actions #13

Updated by Teng Liang about 6 years ago

What's the argument and format of delegation list in the command line? E.g.,

ndnpeek --delegation-list {preference=1,name=/A;preference=2,name=/B} ndn:/name
Actions #14

Updated by Junxiao Shi about 6 years ago

ndnpeek --delegation-list {preference=1,name=/A;preference=2,name=/B} ndn:/name

This syntax is acceptable. I’d prefer a shorter syntax:

--fh /A:1,/B:2
Actions #15

Updated by Davide Pesavento about 6 years ago

No need to invent special syntax for multiple hints. IIRC boost::program_options supports options that are vectors of things: you can specify the option multiple times on the command line and all values are merged together.

Actions #16

Updated by Teng Liang about 6 years ago

  • Assignee changed from Teng Liang to Md Ashiqur Rahman
Actions #17

Updated by Md Ashiqur Rahman about 6 years ago

I'm trying to take the delegations as vector of strings in the command line:

("delegation-list,dl", po::value< std::vector<std::string> >(),
        "set Delegations of the ForwardingHint")

How can I load the vector of strings to the link in options? e.g.

options.link = [something-here](vm["delegation-list"].as< std::vector<std::string> >());

Currently the --link-file option uses io::load taking the filename in string and converts the input from the file to blocks:

options.link = io::load<Link>(vm["link-file"].as<std::string>());
Actions #18

Updated by Davide Pesavento over 5 years ago

  • Subject changed from peek: set forwarding hint to ndnpeek: set forwarding hint
Actions #19

Updated by Davide Pesavento over 5 years ago

  • Priority changed from High to Normal
Actions #20

Updated by Junxiao Shi over 2 years ago

  • Status changed from New to Code review
  • Assignee changed from Md Ashiqur Rahman to Junxiao Shi
  • % Done changed from 0 to 100
Actions #21

Updated by Davide Pesavento over 2 years ago

  • Status changed from Code review to Closed
Actions

Also available in: Atom PDF