Skip to content

Conversation

shun159
Copy link
Contributor

@shun159 shun159 commented Nov 10, 2022

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.

Copy link
Contributor

@that1guy15 that1guy15 left a 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}'
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@that1guy15

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)

Copy link
Contributor

@that1guy15 that1guy15 Dec 23, 2022

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

Copy link
Contributor Author

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?

Copy link
Contributor Author

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.

@shun159 shun159 force-pushed the feature/blueprint_get_node_relationships branch from b670568 to 9bc3df2 Compare December 23, 2022 05:51
Copy link
Contributor

@that1guy15 that1guy15 left a comment

Choose a reason for hiding this comment

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

LGTM

@that1guy15 that1guy15 merged commit dbf0dd2 into Apstra:main Dec 23, 2022
@shun159 shun159 deleted the feature/blueprint_get_node_relationships branch December 23, 2022 15:45
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.

2 participants