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

2592/separate celery #2773

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

2592/separate celery #2773

wants to merge 46 commits into from

Conversation

George-Hudson
Copy link

Summary of Changes

This PR uses terraform to create a redis service and an celery app instance per environment instead of hosting all that on the backend app instance. It updates environment variables in order to connect to the new service for both the backend and celery apps.

How to Test

List the steps to test the PR

When deployed to an environment, it should create the redis servers for the space and celery instance for the env. Both the backend app and the celery should be able to communicate with the redis service for it's environment.

Local testing should remain the same.

cd tdrs-frontend && docker-compose -f docker-compose.yml -f docker-compose.local.yml up -d
cd tdrs-backend && docker-compose -f docker-compose.yml -f docker-compose.local.yml up -d 
  1. Open http://localhost:3000/ and sign in.
  2. Proceed with functional tests as described herein.
  3. Test steps should be captured in the demo GIF(s) and/or screenshots below.

Demo GIF(s) and screenshots for testing procedure

Deliverables

More details on how deliverables herein are assessed included here.

Deliverable 1: Accepted Features

Checklist of ACs:

  • [insert ACs here]
  • lfrohlich and/or adpennington confirmed that ACs are met.

Deliverable 2: Tested Code

  • Are all areas of code introduced in this PR meaningfully tested?
    • If this PR introduces backend code changes, are they meaningfully tested?
    • If this PR introduces frontend code changes, are they meaningfully tested?
  • Are code coverage minimums met?
    • Frontend coverage: [insert coverage %] (see CodeCov Report comment in PR)
    • Backend coverage: [insert coverage %] (see CodeCov Report comment in PR)

Deliverable 3: Properly Styled Code

  • Are backend code style checks passing on CircleCI?
  • Are frontend code style checks passing on CircleCI?
  • Are code maintainability principles being followed?

Deliverable 4: Accessible

  • Does this PR complete the epic?
  • Are links included to any other gov-approved PRs associated with epic?
  • Does PR include documentation for Raft's a11y review?
  • Did automated and manual testing with iamjolly and ttran-hub using Accessibility Insights reveal any errors introduced in this PR?

Deliverable 5: Deployed

  • Was the code successfully deployed via automated CircleCI process to development on Cloud.gov?

Deliverable 6: Documented

  • Does this PR provide background for why coding decisions were made?
  • If this PR introduces backend code, is that code easy to understand and sufficiently documented, both inline and overall?
  • If this PR introduces frontend code, is that code easy to understand and sufficiently documented, both inline and overall?
  • If this PR introduces dependencies, are their licenses documented?
  • Can reviewer explain and take ownership of these elements presented in this code review?

Deliverable 7: Secure

  • Does the OWASP Scan pass on CircleCI?
  • Do manual code review and manual testing detect any new security issues?
  • If new issues detected, is investigation and/or remediation plan documented?

Deliverable 8: User Research

Research product(s) clearly articulate(s):

  • the purpose of the research
  • methods used to conduct the research
  • who participated in the research
  • what was tested and how
  • impact of research on TDP
  • (if applicable) final design mockups produced for TDP development

Copy link

codecov bot commented Jan 9, 2024

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (ae0a5fc) 93.67% compared to head (388411a) 93.76%.
Report is 3 commits behind head on develop.

❗ Current head 388411a differs from pull request most recent head 10cadcd. Consider uploading reports for the commit 10cadcd to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2773      +/-   ##
===========================================
+ Coverage    93.67%   93.76%   +0.09%     
===========================================
  Files          262      219      -43     
  Lines         6053     5087     -966     
  Branches       503      348     -155     
===========================================
- Hits          5670     4770     -900     
+ Misses         287      254      -33     
+ Partials        96       63      -33     
Flag Coverage Δ
dev-backend 93.76% <93.46%> (-0.07%) ⬇️
dev-frontend ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
tdrs-backend/tdpservice/parsers/fields.py 88.63% <ø> (ø)
tdrs-backend/tdpservice/parsers/row_schema.py 94.33% <100.00%> (+0.86%) ⬆️
...s-backend/tdpservice/parsers/schema_defs/header.py 100.00% <ø> (ø)
...s-backend/tdpservice/parsers/schema_defs/ssp/m1.py 100.00% <100.00%> (ø)
...s-backend/tdpservice/parsers/schema_defs/ssp/m2.py 100.00% <100.00%> (ø)
...s-backend/tdpservice/parsers/schema_defs/ssp/m3.py 100.00% <100.00%> (ø)
...s-backend/tdpservice/parsers/schema_defs/ssp/m4.py 100.00% <100.00%> (ø)
...s-backend/tdpservice/parsers/schema_defs/ssp/m5.py 100.00% <100.00%> (ø)
...s-backend/tdpservice/parsers/schema_defs/ssp/m6.py 100.00% <100.00%> (ø)
...s-backend/tdpservice/parsers/schema_defs/ssp/m7.py 100.00% <100.00%> (ø)
... and 25 more

... and 44 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92b0a69...10cadcd. Read the comment docs.

Copy link

@jtimpe jtimpe left a comment

Choose a reason for hiding this comment

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

minor question about some formatting changes - otherwise everything looks and works great!

scripts/deploy-backend.sh Show resolved Hide resolved
@jtimpe jtimpe requested a review from raftmsohani January 17, 2024 19:00
@@ -39,6 +39,6 @@ variable "cf_user" {

variable "dev_app_names" {
type = list(string)
description = "list of app names deployed in the dev environment"
description = "list of app names deployed in the dev cf space"
default = ["a11y", "qasp", "raft", "sandbox"]
Copy link

Choose a reason for hiding this comment

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

have one redis service per environment (dev, staging, prod) vs one per backend (use channel filter)


# bind to redis
cf bind-service "$CGAPPNAME_BACKEND" "tdp-redis-${ENV}"
cf bind-service "$CGAPPNAME_CELERY" "tdp-redis-${ENV}"
Copy link

Choose a reason for hiding this comment

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

env -> space

cf-space:
default: tanf-dev
type: string
environment:

Choose a reason for hiding this comment

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

does this mean it only deploys for raft? or it's only the default

@raftmsohani
Copy link

raftmsohani commented Jan 18, 2024

this is opening up celery monitoring tool (flower) to public so we will need to public:https://tdp-celery-sandbox.app.cloud.gov as it is configured. Additionally, need to add commands to documentation, needed to create route, etc

resource "cloudfoundry_service_instance" "redis" {
name = "tdp-redis-prod"
space = data.cloudfoundry_space.space.id
service_plan = data.cloudfoundry_service.redis.service_plans["PLACEHOLDER"]
Copy link

Choose a reason for hiding this comment

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

select a service plan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Needs to be triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants