gateway allocation cleanup

[codecov[bot]]

Codecov Report

Merging #900 (eaba2c6) into master (10b75ca) will decrease coverage by 0.06%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #900      +/-   ##
==========================================
- Coverage   82.02%   81.96%   -0.07%     
==========================================
  Files          44       44              
  Lines        5481     5495      +14     
==========================================
+ Hits         4496     4504       +8     
- Misses        985      991       +6     

Flag	Coverage Δ
ata	62.68% <70.00%> (+0.06%)	:arrow_up:
aux	68.25% <70.00%> (-0.02%)	:arrow_down:
base	66.19% <70.00%> (+0.01%)	:arrow_up:
dhcp	67.51% <70.00%> (+0.01%)	:arrow_up:
many	67.36% <70.00%> (+0.01%)	:arrow_up:
mud	72.34% <76.66%> (-0.10%)	:arrow_down:
switch	67.49% <76.66%> (+0.02%)	:arrow_up:
topo	66.47% <76.66%> (-0.03%)	:arrow_down:
unit	31.24% <73.33%> (+0.24%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
daq/runner.py	84.87% <100.00%> (+0.23%)	:arrow_up:
daq/session_server.py	87.70% <100.00%> (ø)
daq/topology.py	97.32% <100.00%> (ø)
daq/traffic_analyzer.py	86.50% <0.00%> (-3.18%)	:arrow_down:
daq/acl_state_collector.py	82.19% <0.00%> (-1.37%)	:arrow_down:
daq/host.py	90.98% <0.00%> (-0.15%)	:arrow_down:

---

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 10b75ca...eaba2c6. Read the comment docs.

[grafnu]

At this level in the code there's no explicit recognition that "config is not a protobuf" -- that's a true statement in the current implementation, but I don't find it a particularly compelling argument. I don't know what the statement "not have any override for values that don't make sense" means. The question at hand is "how do you override something to 'default' behavior" -- in the previous version, you could override to default by specifying 0 (which is a nonsensical value for "max hosts" so is semantically unique" -- so really that's my question (proposed mechanism for a config file to override an inherited config).

On Tue, Aug 3, 2021 at 10:27 AM henry54809 @.***> wrote:

@.**** commented on this pull request.

In daq/runner.py https://github.com/faucetsdn/daq/pull/900#discussion_r681961001:

@@ -192,7 +198,7 @@ def init(self, config): self._cleanup_previous_runs() self._init_test_list() self._target_set_queue = []

• self._max_hosts = self.run_trigger.get('max_hosts') or float('inf')
• self._max_hosts = self.run_trigger.get('max_hosts', float('inf'))

This config is not a protobuf object so there is no default 0 value for max_hosts. I think it's best to not have any override for values that don't make sense(in your case, a negative value for max_hosts is also not override). We need to expand the validation of config in the future so daq can just throw an error if some config values don't make sense.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/faucetsdn/daq/pull/900#discussion_r681961001, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIEPDYA3XM3CXFSSEWN7CDT3ARG7ANCNFSM5BPAIR7A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

[henry54809]

PTAL. I restored that max_hosts. we can push off that discussion until we are ready to refactor configs.