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

DM-38718: Arq Queue for Slack #94

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

DM-38718: Arq Queue for Slack #94

wants to merge 1 commit into from

Conversation

Fireye04
Copy link
Member

No description provided.

@Fireye04 Fireye04 force-pushed the tickets/DM-38718 branch 2 times, most recently from b37f69d to 9cfbd35 Compare April 21, 2023 19:47
Copy link
Member

@jonathansick jonathansick left a comment

Choose a reason for hiding this comment

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

Another note that this first commit kind of conflates two separate things. One is setting up arq, which is a redis-based queue. Another is setting up a redis client. Both use Redis, but they're actually separate things. We'll use the redis client to persist state.

requirements/dev.in Show resolved Hide resolved
requirements/main.in Outdated Show resolved Hide resolved
src/semaphore/worker/main.py Outdated Show resolved Hide resolved
src/semaphore/worker/main.py Outdated Show resolved Hide resolved
src/semaphore/worker/main.py Outdated Show resolved Hide resolved
src/semaphore/worker/main.py Show resolved Hide resolved
src/semaphore/worker/main.py Outdated Show resolved Hide resolved
src/semaphore/worker/main.py Outdated Show resolved Hide resolved
src/semaphore/worker/main.py Show resolved Hide resolved
src/semaphore/worker/main.py Outdated Show resolved Hide resolved
Copy link
Member

@jonathansick jonathansick left a comment

Choose a reason for hiding this comment

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

To solve the CI fails, I think you'll need to add the tox-docker plugin. You can do that by modified the tox step in .github/workflows/ci.yaml to have a tox step that includes a tox-plugins line like this:

      - name: Run tox
        uses: lsst-sqre/run-tox@v1
        with:
          python-version: ${{ matrix.python }}
          tox-envs: 'lint,typing,py,coverage-report'
          tox-plugins: "tox-docker"

src/semaphore/config.py Outdated Show resolved Hide resolved
src/semaphore/config.py Outdated Show resolved Hide resolved
src/semaphore/config.py Outdated Show resolved Hide resolved
@Fireye04 Fireye04 force-pushed the tickets/DM-38718 branch 4 times, most recently from 10a3e4f to bd599cf Compare April 21, 2023 23:11
@Fireye04
Copy link
Member Author

Made the suggested changes to ci.yaml, however it continues to fail, citing the following error, then timing out.

WARNING arq.connections:connections.py:266 redis connection error localhost:6379 ConnectionError Error connecting to localhost:6379. Multiple exceptions: [Errno 111] Connect call failed ('::1', 6379, 0, 0), [Errno 111] Connect call failed ('127.0.0.1', 6379).

@Fireye04 Fireye04 force-pushed the tickets/DM-38718 branch 5 times, most recently from b6bfefa to 2b380a1 Compare April 28, 2023 16:35
tox.ini Show resolved Hide resolved
tox.ini Show resolved Hide resolved
Copy link
Member

@jonathansick jonathansick left a comment

Choose a reason for hiding this comment

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

Good start on this. So next you'll want to integrate this building block into the feature spec of sending Slack messages when there are new active messages for this environment.

One approach that you might take is to set up a cron task with Arq. The algorithm for that task is something like:

  1. Compute the active broadcasts (like the web endpoint does)
  2. Get the pre-exisiting state of active broadcasts.
  3. If there are new active broadcasts compared to the state, send those to Slack.
  4. Store the current active messages for the next cron iteration.

Probably the easiest way to store this state is in Redis with https://safir.lsst.io/user-guide/pydantic-redis.html

For that, you'll need to design a Pydantic class that describes the active broadcast messages. You can base that off the Pydantic models used by the web endpoint, like

class BroadcastMessageModel(BaseModel):

requirements/dev.in Outdated Show resolved Hide resolved
src/semaphore/worker/functions/send_message.py Outdated Show resolved Hide resolved
@Fireye04 Fireye04 force-pushed the tickets/DM-38718 branch 2 times, most recently from 2c6137a to 257ebb4 Compare April 28, 2023 21:48
@Fireye04 Fireye04 force-pushed the tickets/DM-38718 branch 3 times, most recently from 32e4029 to d0f9512 Compare May 24, 2023 22:35
@Fireye04 Fireye04 force-pushed the tickets/DM-38718 branch 5 times, most recently from b3ef91b to 1ebd2a3 Compare June 9, 2023 23:22
@Fireye04
Copy link
Member Author

Fireye04 commented Jun 9, 2023

Alright, so getting back into this, I found the documentation for mock slack webhooks, and decided to go about trying to implement it, with limited success. I believe I need to use the post_webhook function to log calls to the webhook, however I'm not certain how I'm supposed to get a request to put into it.

@Fireye04 Fireye04 changed the title DM-38718: Slack Broadcast System DM-38718: Arq Queue for Slack Jun 14, 2023
@Fireye04 Fireye04 force-pushed the tickets/DM-38718 branch 4 times, most recently from 8ac0839 to bfbece6 Compare June 15, 2023 20:04
Add redis and safir dependencies to the project

Add redis to tests

Co-authored-by: Jonathan Sick <[email protected]>
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