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

Wy/wakeup actions lambda #6

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Wynand
Copy link

@Wynand Wynand commented Jul 14, 2021

This adds some functionality to get all the workflows for an organization, and to filter for the ones disabled by inactivity

It also starts to separate logic for retrieving arguments used by KeepAlive so that in the future it will be easy to add support for multiple platforms

@mattBrzezinski mattBrzezinski self-requested a review July 14, 2021 17:25
@Wynand Wynand changed the title Draft: Wy/wakeup actions lambda Wy/wakeup actions lambda Jul 14, 2021
@Wynand
Copy link
Author

Wynand commented Jul 14, 2021

Does not completely close issue #2 (changed to avoid automatically closing the issue)

src/keep_actions_alive.py Outdated Show resolved Hide resolved
@mattBrzezinski
Copy link
Member

closes #2

This PR doesn't completely close #2 as we still want functionality for non-organizational repos to be re-enabled.

src/keep_actions_alive.py Outdated Show resolved Hide resolved
src/keep_actions_alive.py Outdated Show resolved Hide resolved
@Wynand
Copy link
Author

Wynand commented Jul 15, 2021

I seem to have re-enabled all our timed out workflows with a PAT that doesn't have permissions to do that, so I'm not sure how we'll verify this now 🤔

Also the auth parameter on requests.put doesn't accept PATs on their own but we can add headers={'Authorization': f"Bearer {login_or_token}" as an argument instead

@mattBrzezinski
Copy link
Member

I seem to have re-enabled all our timed out workflows with a PAT that doesn't have permissions to do that, so I'm not sure how we'll verify this now 🤔

Also the auth parameter on requests.put doesn't accept PATs on their own but we can add headers={'Authorization': f"Bearer {login_or_token}" as an argument instead

What were the PAT permissions? And for who were they generated?

@Wynand
Copy link
Author

Wynand commented Jul 15, 2021

I seem to have re-enabled all our timed out workflows with a PAT that doesn't have permissions to do that, so I'm not sure how we'll verify this now 🤔
Also the auth parameter on requests.put doesn't accept PATs on their own but we can add headers={'Authorization': f"Bearer {login_or_token}" as an argument instead

What were the PAT permissions? And for who were they generated?

Here are the permissions on the token I'm using, which I generated for my own account:
image

import pprint
import requests

IS_LAMBDA = bool(os.getenv("AWS_EXECUTION_ENV"))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
IS_LAMBDA = bool(os.getenv("AWS_EXECUTION_ENV"))


# Get keyword arguments from environment variables
return {
"login_or_token": os.getenv("GH_LOGIN_OR_TOKEN"),
Copy link
Member

Choose a reason for hiding this comment

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

This should not be an environment variable. It needs to be stored in SecretsManager and using boto to retrieve it.


# Get all repos that are not forks and are not archived
# Only supports repos owned by an organization for now,
# but could be changed to users
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# but could be changed to users
# but could be changed to users
# TODO: Support non-organizational repositories

@@ -0,0 +1,30 @@
#!/usr/bin/env python
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the point of this file, why not just have everything in one script?

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