-
Notifications
You must be signed in to change notification settings - Fork 13
blueprint: Add support for GET /api/blueprints/{bp_id}/relationships #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
blueprint: Add support for GET /api/blueprints/{bp_id}/relationships #44
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment and you need to rebase and bump version #.
Looks good, thank you!
aos/blueprint.py
Outdated
|
||
if queries: | ||
query_str = '&'.join(queries) | ||
url += f'?{query_str}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to append the url, you can use the queries in the header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your review!
Please tell me how I can put queries in header? (I don't know how to do except for append into URL)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you look at the json_resp_get() method you will see it takes params
as an optional argument as a dictionary with your query parameters as key/value pairs. This library is designed to take params and pass them in through the headers when populated so you dont have to work with them raw in the url
There are examples through out this library that are using the params
argument in the rest class. Here is a good example: https://github.com/Apstra/apstra-api-python/blob/main/aos/blueprint.py#L536
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good advice! I have fixed to use params instead of querystring. How about that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And rebased to bump version number.
b670568
to
9bc3df2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Add support for GET
/api/blueprints/{bp_id}/relationships
This API is useful in some use cases like finding a pair of interface and device.