-
Notifications
You must be signed in to change notification settings - Fork 79
feature:) slackapp approval integration #1715
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
base: KC-978/slackapp-integration
Are you sure you want to change the base?
feature:) slackapp approval integration #1715
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| print(f" URL: {self.config.keeper.service_url}") | ||
| print(" The app will start but commands may fail.\n") | ||
|
|
||
| # Start PEDM poller in background |
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.
Remove the commented code
| """ | ||
| Initialize Keeper Slack App. | ||
| """ | ||
| print("[INFO] Initializing Keeper Commander Slack App...") |
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.
Replace all print() with proper logger
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.
Do we need this log file?
| timeout=10 | ||
| ) | ||
|
|
||
| if response.status_code != 202: |
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.
Code from line 71 to 83 is repeated in each function. Make a common function for it use it in each API call function
| pass | ||
| else: | ||
| print(f"[WARN] Unknown status: {status}") | ||
| else: |
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.
Need to handle 429 API response status code
No description provided.