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.

Sign up for GitHub

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

Jump to bottom

[VAMC-19452]Update Alternative Banners DB every 10min with sidekiq job #19550

Merged
merged 7 commits into from
Dec 2, 2024
Merged

[VAMC-19452]Update Alternative Banners DB every 10min with sidekiq job #19550

merged 7 commits into from
Dec 2, 2024

Conversation

SnowboardTechie
Copy link
Contributor

@SnowboardTechie SnowboardTechie commented Nov 21, 2024 •
edited
Loading

Summary

  • This work is behind a feature toggle (flipper): YES
  • This work introduces a new sidekiq job to utilize the Banner DB updater in [VACMS-19451]Alternative Banners fetch banner data from graphql to db #19511 and update the Banner DB every 10 minutes
  • Sitewide Facilities team component
  • Flipper is being used as a safety net to enable the quick disabling of the regular job if any issues are noticed upon activation

Related issue(s)

Testing done

  • New code is covered by unit tests
  • Previously, no job was enqueued to update the DB and Banners DB is empty. When enabled, this will update the DB every 10 minutes.
  • If this work is behind a flipper:
    • Yes, will enable flipper once live and monitor DB and logs for updates processing without issue.

Screenshots

Note: Optional

What areas of the site does it impact?

Nowhere visible to users ATT, it is updating the DB in preparation to display situation updates for facilities to users when viewing VAMC data.

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

Requested Feedback

(OPTIONAL)What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?

@SnowboardTechie SnowboardTechie self-assigned this Nov 21, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 21, 2024 •
edited
Loading

1 Warning
⚠️ This PR changes 360 LoC (not counting whitespace/newlines).

In order to ensure each PR receives the proper attention it deserves, we recommend not exceeding
200. Expect some delays getting reviews.

File Summary

Files

  • .github/CODEOWNERS (+0/-1)

  • app/controllers/v0/banners_controller.rb (+7/-5)

  • config/features.yml (+3/-0)

  • lib/periodic_jobs.rb (+1/-0)

  • modules/banners/app/models/banner.rb (+2/-3)

  • modules/banners/app/sidekiq/banners/update_all_job.rb (+44/-0)

  • modules/banners/lib/banners.rb (+8/-0)

  • modules/banners/lib/banners/builder.rb (+30/-0)

  • modules/banners/lib/banners/profile/vamc.rb (+21/-0)

  • modules/banners/lib/banners/updater.rb (+62/-0)

  • modules/banners/spec/lib/builder_spec.rb (+79/-0)

  • modules/banners/spec/lib/updater_spec.rb (+80/-0)

  • modules/banners/spec/models/banner_spec.rb (+5/-5)

  • spec/models/banner_spec.rb (+0/-4)

    Note: We exclude files matching the following when considering PR size:

    *.csv, *.json, *.tsv, *.txt, *.md, Gemfile.lock, app/swagger, modules/mobile/docs, spec/fixtures/, spec/support/vcr_cassettes/, modules/mobile/spec/support/vcr_cassettes/, db/seeds, modules/vaos/app/docs, modules/meb_api/app/docs, modules/appeals_api/app/swagger/, *.bru, *.pdf

Big PRs are difficult to review, often become stale, and cause delays.

Generated by 🚫 Danger

@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@SnowboardTechie SnowboardTechie force-pushed the VAMC-19452-alternative-banners-sidekiq-job branch from 6b2b29b to 461c7da Compare November 21, 2024 18:45
@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@SnowboardTechie SnowboardTechie force-pushed the VAMC-19452-alternative-banners-sidekiq-job branch from 461c7da to 0321fc9 Compare November 21, 2024 18:48
@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@SnowboardTechie SnowboardTechie force-pushed the VAMC-19452-alternative-banners-sidekiq-job branch from 0321fc9 to 0f63330 Compare November 21, 2024 18:53
@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@SnowboardTechie SnowboardTechie force-pushed the VAMC-19452-alternative-banners-sidekiq-job branch 2 times, most recently from e7c249d to 8421ed8 Compare November 21, 2024 18:56
@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@SnowboardTechie SnowboardTechie force-pushed the VAMC-19452-alternative-banners-sidekiq-job branch 2 times, most recently from 8bb9edf to 1651f1f Compare November 21, 2024 19:12
@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@SnowboardTechie SnowboardTechie force-pushed the VAMC-19452-alternative-banners-sidekiq-job branch 2 times, most recently from 61f4d46 to e47fe7b Compare November 25, 2024 16:32
@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@github-actions GitHub Actions
Copy link

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@SnowboardTechie
Copy link
Contributor Author

Okay, there are too many of these empty comments, I give up cleaning them up.

@SnowboardTechie
Copy link
Contributor Author

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

I think I figured it out, this is because the check is working against master which will not contain the CODEOWNER update until the base branch of this PR is merged into master

eselkin
eselkin approved these changes Nov 25, 2024
View reviewed changes
Copy link
Contributor

@eselkin eselkin left a comment

Choose a reason for hiding this comment

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

LGTM

@platform-support-slack-integration platform-support-slack-integration bot mentioned this pull request Dec 2, 2024
Closed
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 2, 2024

Backend-review-group approval confirmed.

@SnowboardTechie SnowboardTechie merged commit 0fc8f38 into VACMS-19449-situation-updates-fetch-banner-data Dec 2, 2024
22 of 25 checks passed
@SnowboardTechie SnowboardTechie deleted the VAMC-19452-alternative-banners-sidekiq-job branch December 2, 2024 20:44
SnowboardTechie added a commit that referenced this pull request Dec 2, 2024
@SnowboardTechie @eselkin
[VACMS-19451]Alternative Banners fetch banner data from graphql to db (…
4d6a935 
…#19511)

* add initial thougnts/notes

* create banner job to pull and update db, only fetching banner data

* cleanup notes and variable names

* Move fetching out of main job into banner services for building and updating

* Further refinement of builder/updater/job process

* get builder creating banners using parsed response data

* params refinement

* rubocop cleanup

* separate banner profiles from updater

* adjust scope to simplify queries a notch for easier understanding

* add error response to controller when path is not provided for #by_path

* remove no longer used #enabled?

* cleanup updater

* cleanup requirements

* appease the rubocop

* destroy any lingering banners that are no longer being provided

* fixup query and vamc model naming

* pluck > map

* remove job to be handled with VAMC-19452

* remove leftover banner spec and adjust #by_path testing in appropriate banner spec

* add logging to builder and updater

* add updater spec

* add spec for builder

* update settings used, appease the rubocop

* avoid alternative_banners wording

* missed a spec

* rebasing added the job too soon

* return when rendering error

* 422 is more accurate than 400

* [VAMC-19452]Update Alternative Banners DB every 10min with sidekiq job (#19550)

* adjust updater to return error when parsing failed, introduce job to work with updater

* avoid alternative_banners wording

* add job to 10m rotation

* add flipper to enable the update all job

* make linter happy

* be more specific when re-raising error

* update updater spec to include error

* Use Job in periodic_jobs.rb

Co-authored-by: Eli Selkin <[email protected]>

---------

Co-authored-by: Eli Selkin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rjohnson2011 rjohnson2011 rjohnson2011 approved these changes

@eselkin eselkin eselkin approved these changes

Assignees

@SnowboardTechie SnowboardTechie

Labels
codeowners-addition-failure codeowners-delete-failure require-backend-approval test-passing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SnowboardTechie @eselkin @rjohnson2011 @va-vfs-bot