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

feat(isobot): add locking/cloning functionalities #1392

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

seaerchin
Copy link
Contributor

@seaerchin seaerchin commented May 28, 2024

Problem

We want to shift locking/unlocking functionality into a slack bot because using the form is unwieldy. This is because we have to log into form sg -> otp -> find form -> otp again.

Solution

  • just shift related APIs/endpoint into a slackbot endpoint. copied stuff as-is from the existing form repair service so there shouldn't be too much differences
  • not quite sure if it's safe to set error.cause in unlock for mutex-utils. as is, the current return from unlock is pretty useless

@seaerchin seaerchin changed the title feat(isobot): add locking/repair functionalities feat(isobot): add locking/cloning functionalities May 28, 2024
@seaerchin seaerchin marked this pull request as ready for review May 28, 2024 13:00
@seaerchin seaerchin requested a review from a team May 28, 2024 13:00
@seaerchin seaerchin marked this pull request as draft May 28, 2024 17:12
Base automatically changed from ref/isobolt to develop June 5, 2024 10:15
add whitelisting
add endpoints for cloning and locking
change to class
shift to using the class
@seaerchin seaerchin marked this pull request as ready for review June 28, 2024 07:32
@@ -16,11 +23,37 @@ const token = config.get("slackbot.token")
const botReceiver = new ExpressReceiver({ signingSecret, endpoints: "/" })
export const isobotRouter = botReceiver.router

// TODO: add slack ids of isomer user
const ISOMER_USERS_ID = ["U01HTSFC0RY"]
Copy link
Contributor

Choose a reason for hiding this comment

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

should we be hardcoding this? Or fetch it dynamically? Else every member movement in team will require a change

An alternative I can think of is to fetch members of the isomer-team and then check if the user is within this team. I believe the team will be more permanent than a user id

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok this sounds good to me, i'll make this change!

await ack()

const HELP_TEXT =
"Usage: `/clone <github_repo_name>`. Take note that this locks the repo for 15 minutes by default. To bypass this behaviour, add `-n` at the end of the command"
Copy link
Contributor

Choose a reason for hiding this comment

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

us clone the right term? we usually refer to it as a repair?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the repair does a lock -> clone -> unlock under the hood!

Copy link
Contributor

Choose a reason for hiding this comment

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

correct, hence the 3-step process is called repair

await ack()

const HELP_TEXT =
"Usage: `/lock <github_repo_name> -d <duration_in_minutes>`. Take note that this locks the repo for 15 minutes by default if `-d` is not specified"
Copy link
Contributor

Choose a reason for hiding this comment

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

should d also have a max time? what if someone puts in an accidental 10000000 mins or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense, will make this change

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