Conversation
hardikdr
added
the
area/metal-automation
felix-kaestner
force-pushed
the
main
branch
2 times, most recently
from
64984b9 to
f71555b
Compare
sven-rosenzweig
force-pushed
the
feat/bundleiface
branch
from
fbec789 to
3708c30
Compare
felix-kaestner
force-pushed
the
main
branch
4 times, most recently
from
aed4596 to
8d57167
Compare
sven-rosenzweig
force-pushed
the
feat/bundleiface
branch
from
3708c30 to
65eaaf9
Compare
In Cisco IOS XR the yang empty type is not implemented correctly for bundle-interfaces. Instead of returning "[null]" as defined in the RFC, "[\n null \n]" is. We simply work around of this.
For Bundle- and Bundlesubinterfaces creation fails, as the gnmi Update/Patch call, checks whether the object exists or not. We skip this check once if the gnmi call returns a NotFound error code.
Introduce new API field InnerVlan storing the inner vlan of a QandQ Tag.
sven-rosenzweig
force-pushed
the
feat/bundleiface
branch
from
65eaaf9 to
ef97c0e
Compare
sven-rosenzweig
force-pushed
the
feat/bundleiface
branch
from
ef97c0e to
6789276
Compare
There was a problem hiding this comment.
Can you please invoke make charts to also regenerate the Helm Chart incl. this CRD.
| if string(b) != "[null]" { | ||
|
|
||
| // Due to some Cisco IOSX ouptut we also match [ \n null \n] | ||
| nullTypeRe := regexp.MustCompile(`^\[\s*null\s*]$`) |
There was a problem hiding this comment.
nit: can you move this variable outside of the function, so that we don't recreated this regexp instance on every function call?
| EtherBundle IFaceSpeed = "etherbundle" | ||
| ) | ||
|
|
||
| type BunldePortActivity string |
| StateShutDown PhysIfStateType = "im-state-shutdown" | ||
| ) | ||
|
|
||
| // Represent physical and bundle interfaces as part of the same struct as they share a lot of common configuration |
There was a problem hiding this comment.
| // Represent physical and bundle interfaces as part of the same struct as they share a lot of common configuration | |
| // Iface represents physical and bundle interfaces as part of the same struct as they share a lot of common configuration |
Type comments should start with the type name.
| } | ||
| if string(b) != "[null]" { | ||
|
|
||
| // Due to some Cisco IOSX ouptut we also match [ \n null \n] |
There was a problem hiding this comment.
| // Due to some Cisco IOSX ouptut we also match [ \n null \n] | |
| // Due to some Cisco IOS-XR output we also match [ \n null \n] |
| func (m *MockClient) Create(ctx context.Context, conf ...gnmiext.Configurable) error { | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
I think this was left by mistake and should be removed.
There was a problem hiding this comment.
Please don't introduce a util package for reasons explained in the Google Go Styleguide 1.
At the same time, as both functions are pretty simple and could be just oneline statements, I would just inline them in the only place they are currently used.
Footnotes
-
https://google.github.io/styleguide/go/decisions#naming:~:text=Avoid%20uninformative%20package%20names%20like%20util%2C%20utility%2C%20common%2C%20helper%2C%20model%2C%20testhelper%2C%20and%20so%20on%20that%20would%20tempt%20users%20of%20the%20package%20to%20rename%20it%20when%20importing%2E%20See. ↩
3 participants
No description provided.