akarneliuk / pygnmi

The pure Python implementation of the gNMI client.
https://training.karneliuk.com
BSD 3-Clause "New" or "Revised" License
129 stars 44 forks source link

gnmi enhancement to support json_ietf encoding and work when json_ietf_val is bytes string #58

Closed brunoonovais closed 2 years ago

brunoonovais commented 2 years ago

json_ietf_val = b'DIRECTLY_CONNECTED'

This could be a simple fix as well, which I've made locally and it works.

Thank you for the awesome library!

akarneliuk commented 2 years ago

Hey @brunoonovais , thanks for your message. I'm not too sure I do understand the request, as pygnmi already supports all encodings, which are supported by gNMI: https://github.com/akarneliuk/pygnmi/blob/903e50bc11b38dbb98d03d549e67908cc88da2e1/pygnmi/client.py#L272

On a second one, it would be good to see, what was the request you have so far performed to receive such an answer. Given it is a DIRECTLY_CONNECTED, I would assume you have requested for a specific route. However, it would be good to see the message, which caused an exception, if possible.

Overall, if you feel that something can be improved, you are welcome to raise a pull request ;-)

Best, Anton

brunoonovais commented 2 years ago

Hey Anton

Sorry, I should've worded this properly. I actually meant pygnmicli, not pygnmi client. I will just submit a PR and you let me know if you have a problem with that or not :) Didn't want to raise it before talking to you first.

Thank you! Bruno Novais

On Sun, May 1, 2022 at 6:54 PM Anton Karneliuk @.***> wrote:

Hey @brunoonovais https://github.com/brunoonovais , thanks for your message. I'm not too sure I do understand the request, as pygnmi already supports all encodings, which are supported by gNMI: https://github.com/akarneliuk/pygnmi/blob/903e50bc11b38dbb98d03d549e67908cc88da2e1/pygnmi/client.py#L272

On a second one, it would be good to see, what was the request you have so far performed to receive such an answer. Given it is a DIRECTLY_CONNECTED, I would assume you have requested for a specific route. However, it would be good to see the message, which caused an exception, if possible.

Overall, if you feel that something can be improved, you are welcome to raise a pull request ;-)

Best, Anton

— Reply to this email directly, view it on GitHub https://github.com/akarneliuk/pygnmi/issues/58#issuecomment-1114356532, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKG3BBD2VKIPPDJPO3BBQFTVH4DSRANCNFSM5UWCXIYA . You are receiving this because you were mentioned.Message ID: @.***>

akarneliuk commented 2 years ago

Hey @brunoonovais ,

sounds good, looking forward.

Best, Anton

brunoonovais commented 2 years ago

Hi Anton

Just wanted to send you the diff before I send a PR. Do you see any problem with this? I may have to change a couple of things on client.py as well, but wanted to send those changes to pygnmicli at least for now.

Thank you Bruno Novais

On Tue, May 3, 2022 at 6:12 PM Anton Karneliuk @.***> wrote:

Hey @brunoonovais https://github.com/brunoonovais ,

sounds good, looking forward.

Best, Anton

— Reply to this email directly, view it on GitHub https://github.com/akarneliuk/pygnmi/issues/58#issuecomment-1116711275, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKG3BBDWHS5XPFX3RP4TJ73VIGQFRANCNFSM5UWCXIYA . You are receiving this because you were mentioned.Message ID: @.***>

akarneliuk commented 2 years ago

Hey @brunoonovais,

sure, please send it to me or submit PR so I can review directly there.

Best, Anton

akarneliuk commented 2 years ago

Hey @brunoonovais ,

hope you are well. Are you still going to submit PR?

Best, Anton

brunoonovais commented 2 years ago

Hey Anton

I've recently changed jobs so could not catch up on this, sorry about that. I will find a box to test the changes I've made again on this other vendor and let you know. It just may take a bit of time since I have a ton of stuff to learn for the job haha.

Cheers! Bruno Novais

On Thu, Jul 14, 2022 at 15:43 Anton Karneliuk @.***> wrote:

Hey @brunoonovais https://github.com/brunoonovais ,

hope you are well. Are you still going to submit PR?

Best, Anton

— Reply to this email directly, view it on GitHub https://github.com/akarneliuk/pygnmi/issues/58#issuecomment-1184830875, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKG3BBDEGAMONCTERWW3NBTVUBUVXANCNFSM5UWCXIYA . You are receiving this because you were mentioned.Message ID: @.***>

akarneliuk commented 2 years ago

Hey @brunoonovais ,

hope you enjoy a new job :-) I believe your request is now sorted by #92 . Could you please test and let me know if anything else is needed?

Best, Anton

brunoonovais commented 2 years ago

Hey Anton, thank you! all good :) I just pulled the latest and still seeing it:

bash-3.2$ ./pygnmicli -t device:6030 -u admin -p admin -i -o capabilities -d all Traceback (most recent call last): File "/Users/bnovais/Downloads/pygnmi/scripts/./pygnmicli", line 163, in

main() File "/Users/bnovais/Downloads/pygnmi/scripts/./pygnmicli", line 40, in main target=args.target, username=args.username, password=args.password, token=args.token, AttributeError: 'Namespace' object has no attribute 'token' bash-3.2$ I think we can just verify if args.token and use that option, otherwise remove it. That is what I did to get around it, but looks clean enough? Cheers! Bruno Novais On Wed, Aug 10, 2022 at 12:22 PM Anton Karneliuk ***@***.***> wrote: > Hey @brunoonovais , > > hope you enjoy a new job :-) I believe your request is now sorted by #92 > . Could you please test > and let me know if anything else is needed? > > Best, > Anton > > — > Reply to this email directly, view it on GitHub > , > or unsubscribe > > . > You are receiving this because you were mentioned.Message ID: > ***@***.***> >
akarneliuk commented 2 years ago

Hello @brunoonovais ,

First of all, I believe it is related to another issue, namely #85 . Second, I so far was not able to reproduce the issue against vEOS. but I will try another go.

Best, Anton

akarneliuk commented 2 years ago

Also, what is interesting. where are you running that from? It doesn't seem you properly install it via pip as otherwise you would run it as pygnmicli without ./. What version of Python do you have on the host?

akarneliuk commented 2 years ago

Hello @brunoonovais ,

Both your requested items are sorted in #93 and published in pygnmi==0.8.8, which is just released in pypi as well. Please, test and let me know.

Best, Anton

akarneliuk commented 2 years ago

Closed due to inactivity