Skip to content
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

Add subscription event source plugin (DCNE-269) #712

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jgomezve
Copy link

No description provided.

@jgomezve
Copy link
Author

Fix #713

@akinross akinross changed the title Add subscription event source plugin Add subscription event source plugin (DCNE-269) Dec 17, 2024
@akinross
Copy link
Collaborator

please run black and fix sanity issues

urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)


def login(hostname: str, username: str, password: str) -> str:
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a reason for not reusing the login functions from the ansible collection? can we reuse connection definitions from modules?

return sub_ids


async def refresh(hostname: str, token: str, refresh_timeout: int, sub_ids: list[str]) -> NoReturn:
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we name the function refresh_subscription

refresh_timeout = int(args.get("refresh_timeout", 300))
subscriptions = args.get("subscriptions")

if "" in [hostname, username, password]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we also support private key as we do with modules? this would avoid the login step

:param sub_ids: subscription ids
:return: NoReturn
"""
cookie = {"APIC-cookie": token}
Copy link
Collaborator

Choose a reason for hiding this comment

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

can this token expire? or would the subscriptionRefresh also refresh the session?

sys.exit(1)

if not isinstance(subscriptions, list) or subscriptions == [] or subscriptions is None:
print(f"subscriptions is empty or not a list: {subscriptions}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

would typically avoid or in errors, can we be more specific which case is triggering the error


:param hostname: apic hostname or ip
:param token: apic session token
:param rf_timeout: refresh timeout of subscription
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the metric of the timeout? seconds I assume?

await asyncio.sleep(refresh_timeout / 2)
for sub_id in sub_ids:
refresh_url = f"https://{hostname}/api/subscriptionRefresh.json?id={sub_id}"
requests.get(refresh_url, verify=False, cookies=cookie, timeout=60)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we do these request also async?

- apic1: APIC IP or hostname
- username: APIC username
- password: APIC password
- subscriptions: query api endpoints for subscriptions
Copy link
Collaborator

Choose a reason for hiding this comment

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

are there limits for amount of subscriptions, should we document this?

An ansible-rulebook event source plugin template.

Arguments:
- apic1: APIC IP or hostname
Copy link
Collaborator

Choose a reason for hiding this comment

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

is apic1 a typo?

@@ -0,0 +1,148 @@
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

please add author details and license info as we do for modules

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