Skip to content

provided client type for PCL to have proper telemetry#169

Merged
xorrkaz merged 4 commits intoCiscoDevNet:masterfrom
vprysiaz:set_client_type
Mar 12, 2026
Merged

provided client type for PCL to have proper telemetry#169
xorrkaz merged 4 commits intoCiscoDevNet:masterfrom
vprysiaz:set_client_type

Conversation

@vprysiaz
Copy link
Contributor

@vprysiaz vprysiaz commented Mar 6, 2026

No description provided.

@xorrkaz
Copy link
Collaborator

xorrkaz commented Mar 6, 2026

Thanks for this @vprysiaz . But all tests fail. Looks like an indentation issue. Moreover, will this work with PCL >= 2.8?

@xorrkaz
Copy link
Collaborator

xorrkaz commented Mar 10, 2026

And by work, I mean will it break PCL >= 2.8 if it's specified? If it is just ignored as a kwargs key, then that's fine.

@vprysiaz
Copy link
Contributor Author

I will fix it today/tomorrow. yes. it is issue in pcl (we do not ignore kwargs...)
as an option we can update pcl version used here. or I will add version check to send this tag only when pcl is higher then version X

virl/helpers.py Outdated
client = ClientLibrary(server.host, server.user, server.passwd,
raise_for_auth_failure=True,
ssl_verify=ssl_verify,
client_type="VirlUtils",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For modernity, this should be "cmlutils"

Copy link
Collaborator

@xorrkaz xorrkaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good in general. I just had one request.

@xorrkaz
Copy link
Collaborator

xorrkaz commented Mar 11, 2026

You also still have an indentation issue. See the test results.

@vprysiaz
Copy link
Contributor Author

You also still have an indentation issue. See the test results.

how to run them locally?
I did pytest tests/ and they all passed. indentation issue can be present as there was no linter run

@vprysiaz
Copy link
Contributor Author

I wanna submit fix for a tag and indentation in one commit.

@vprysiaz vprysiaz requested a review from xorrkaz March 11, 2026 14:58
@xorrkaz xorrkaz merged commit 8c33d42 into CiscoDevNet:master Mar 12, 2026
5 checks passed
@coveralls
Copy link

coveralls commented Mar 12, 2026

Pull Request Test Coverage Report for Build 22954427290

Details

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 78.588%

Totals Coverage Status
Change from base Build 22548103014: 0.03%
Covered Lines: 2070
Relevant Lines: 2634

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants