Support for UMF Interfaces Model - Front Panel Interfaces#229
Support for UMF Interfaces Model - Front Panel Interfaces#229aliyahhoda wants to merge 1 commit into
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
aliyahhoda
force-pushed
the
pr_intfmodel_frontpanel
branch
from
9e4aed3 to
7b6ea0d
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
aliyahhoda
force-pushed
the
pr_intfmodel_frontpanel
branch
from
7b6ea0d to
e3afe8c
Compare
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
aliyahhoda
force-pushed
the
pr_intfmodel_frontpanel
branch
from
e3afe8c to
2a34ca1
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
aliyahhoda
force-pushed
the
pr_intfmodel_frontpanel
branch
from
2a34ca1 to
75f5d4f
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Support for UMF Front Panel Interfaces config/id (P4RT id) Unit Testing Result File: translib/platform/platform_test.go === RUN TestMissingJsonFile |
aliyahhoda
force-pushed
the
pr_intfmodel_frontpanel
branch
from
75f5d4f to
9dc3a0a
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
aliyahhoda
force-pushed
the
pr_intfmodel_frontpanel
branch
from
9dc3a0a to
d0fafff
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| * populate most fields. */ | ||
| platformCfg = platformConfig{} | ||
| platformCfg.intfs = map[string]*platformIntf{} | ||
| intfsJson := parsedJson["interfaces"].(map[string]any) |
There was a problem hiding this comment.
I notice that platform.go uses several unchecked type assertions on parsed JSON (.(map[string]any), .(map[string]interface{}), .([]interface{})). Could we avoid direct type assertion and use checked assertions (v, ok := ...) and return descriptive InvalidArgs/tlerr.New errors instead of crashing?
Same pattern appears in a few other places in this file:
• parsedJson["interfaces"].(map[string]any)
• interface-entry casts in both legacy and sonic parsing paths
• primary-entry cast in second pass
• breakout_modes map cast
• breakout mode members cast to []interface{}
| var intfToDefaultMode = map[string]string{} // from hwsku.json | ||
|
|
||
| /* Functions */ | ||
| func init() { |
There was a problem hiding this comment.
platform currently performs file parsing and global state mutation in init(). This introduces import-time side effects, makes tests order-sensitive, and obscures initialization failure handling. Can we switch to explicit lazy initialization (ensureLoaded() with sync.Once) and return errors at call sites? That keeps startup deterministic, improves testability, and avoids hidden global-state coupling.
e.g.
var (
cfg platformConfig
initErr error
once sync.Once
)
func ensureLoaded() error {
once.Do(func() {
initErr = loadAll() // parse files, validate, then assign cfg
})
return initErr
}
Then every public accessor calls ensureLoaded() first.
| return true, nil | ||
| } | ||
|
|
||
| func doParsePlatformJson(filename string) error { |
There was a problem hiding this comment.
doParsePlatformJson and doParsePlatformJsonSonic share massive blocks of nearly identical code. Can you make common sections extracted into shared helper functions?
| needCache: true, | ||
| onChange: OnchangeDisable, | ||
| dbDataMap: make(RedisDbSubscribeMap), | ||
| nOpts: ¬ificationOpts{mInterval: 1, pType: Sample}, // Counters can only support Sample. |
There was a problem hiding this comment.
The Subscribe_intf_get_counters_xfmr has been significantly refactored:
• Before: Used COUNTERS_PORT_NAME_MAP from CountersDB, with 30s sample interval
• After: Uses config DB tables (PORT, etc.) with 1s sample interval
This is a significant behavioral change that could impact subscription performance. The sample interval dropped from 30s to 1s, which is a 30x increase in polling frequency. The comment says "Counters can only support Sample" but doesn't explain why the interval changed so drastically.
|
|
||
| var Subscribe_intf_get_ether_counters_xfmr SubTreeXfmrSubscribe = func(inParams XfmrSubscInParams) (XfmrSubscOutParams, error) { | ||
| return Subscribe_intf_get_counters_xfmr(inParams) | ||
| log.Info("Entering Subscribe_intf_get_ether_counters_xfmr") |
There was a problem hiding this comment.
Previously this called Subscribe_intf_get_counters_xfmr directly. Now it has its own separate implementation. This means future changes to counter subscription logic need to be applied in two places.
Signed-off-by: Aliyah Hoda <ahoda@google.com>
aliyahhoda
force-pushed
the
pr_intfmodel_frontpanel
branch
from
d0fafff to
ec7dab0
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
3 participants
Support for UMF Interfaces Model - Front Panel Interfaces