Opened 9 years ago

Closed 9 years ago

#201 closed enhancement (fixed)

Cmdctl: add value/type check according to module specification.

Reported by: zhanglikun Owned by: zhanglikun
Priority: medium Milestone: 04. 2nd Incremental Release: Early Adopters
Component: ~cmd-ctl (obsolete) Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: Add Hours to Ticket:
Total Hours: Internal?:

Description

Create this ticket for task 80. The code has been committed to branches/likun-value-check.(revision 1911). After review, the code need to be merged to trunk.

Subtickets

Change History (8)

comment:1 Changed 9 years ago by zhanglikun

  • Owner changed from zhanglikun to jelte
  • Status changed from new to reviewing

Jelte, could you have a review my code(revision 1911), since I did some change to files in lib/python/isc/config.

comment:2 Changed 9 years ago by shane

  • Milestone changed from 03. 1st Incremental Release to 04. 2nd Incremental Release

comment:3 follow-up: Changed 9 years ago by jelte

  • Owner changed from jelte to zhanglikun

It's in pretty good shape, but some initial comments;

  • it seems you have forgotten to add a test data file (spec27) to the repository (so the tests fail now)
  • I think there should be an exception to the check for the 'fake' module "config", whose commands now all fail (and they have their own checks so this one could just pass if the command is a configuration command, i think)

comment:4 in reply to: ↑ 3 Changed 9 years ago by zhanglikun

Replying to jelte:

It's in pretty good shape, but some initial comments;

  • it seems you have forgotten to add a test data file (spec27) to the repository (so the tests fail now)
  • I think there should be an exception to the check for the 'fake' module "config", whose commands now all fail (and they have their own checks so this one could just pass if the command is a configuration command, i think)

Hi Jelte, do you mean don't do value/type check for the command sent to config manager? Yes, all commands sent to config manager are skipped without any check.

comment:5 Changed 9 years ago by zhanglikun

Patch is committed in revision 1944. Ignore value/type check for commands belong to fake module 'config'.

comment:6 Changed 9 years ago by jelte

looks good, tried it and it no longer fails for me.

One note, I see you added a constant CONFIG_MODULE_NAME (good! :) ), there is still one place where "config" is directly used, in the creation of the fake module in bindctl-source.py(.in)

comment:7 Changed 9 years ago by zhanglikun

code has been merged to trunk in r1959.

comment:8 Changed 9 years ago by zhanglikun

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.