Skip to content

Conversation

@loneil
Copy link
Collaborator

@loneil loneil commented Jul 18, 2025

Issue #: /bcgov/entity#29610

Description of changes:

Replace the entity-digital-credentials queue service with a GCP pub/sub business-digital-credentials handler. Add ci/cd handlers (have discussed with Andriy and added necessary vault values in 1password)

This is a net-new service, would not affect any existing queue services or business-api functionality upon merge.

Looks like lots of files to review here but the code under processors where most of the business logic is really just moved over from existing production code in the old entity- version of the queue service.

Uses dependencies from in-review PRs:

  1. 29608 Python/Common Business Model: Digital Credentials DBC Phase 2 additions #3642
  2. 29609 DBC modules in python/common #3644

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the lear license (Apache 2.0).

@loneil loneil force-pushed the feature/businessDigitalCredentialsQueue branch 10 times, most recently from 43627c1 to 507586c Compare July 23, 2025 17:51
@loneil loneil marked this pull request as ready for review July 23, 2025 17:54
Copilot AI review requested due to automatic review settings July 23, 2025 17:54

This comment was marked as outdated.

@loneil loneil requested a review from Copilot July 23, 2025 22:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new Business Digital Credentials GCP queue service to replace the existing entity-digital-credentials service. The service handles digital credential tasks for business events through GCP pub/sub messaging, including business number updates, dissolution filings, change of registration, and administrative revocations.

  • Complete queue service implementation with GCP pub/sub integration and authentication
  • Digital credential processors for various business events (business number, dissolution, change of registration, admin revoke, put back on)
  • Comprehensive test coverage including unit tests for all processors, helpers, and services

Reviewed Changes

Copilot reviewed 47 out of 49 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
queue_services/business-digital-credentials/src/business_digital_credentials/ Core application setup with Flask factory pattern and service initialization
queue_services/business-digital-credentials/src/business_digital_credentials/digital_credential_processors/ Business logic processors for different digital credential events
queue_services/business-digital-credentials/tests/ Comprehensive test suite with unit tests for all components
queue_services/business-digital-credentials/devops/ CI/CD configuration and deployment scripts
.github/workflows/ GitHub Actions workflows for CI/CD


digital_credentials_helpers.log_something()

if msg := verify_gcp_jwt(request):
Copy link
Collaborator

Choose a reason for hiding this comment

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

is verify_gcp_jwt needed? in gcp, it is possible to set 'invoker' permissions on the cloudrun deployment only for pubsub service account used in pushing the notification

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not too sure in general, it appeared to me it is, as the other deployed business queue services do a verify_gcp_jwt when receiving the message. So I don't think the deployment pattern for these has been to set up those permissions?

@severinbeauvais
Copy link
Collaborator

@argush3 , @vysakh-menon-aot , do you have time to review this and comment on Andriy's conversation?

loneil added 3 commits July 28, 2025 12:37
Signed-off-by: Lucas <lucasoneil@gmail.com>

Formating fixes from ruff

Signed-off-by: Lucas <lucasoneil@gmail.com>
Signed-off-by: Lucas <lucasoneil@gmail.com>
Signed-off-by: Lucas <lucasoneil@gmail.com>
@loneil loneil force-pushed the feature/businessDigitalCredentialsQueue branch from 527f3fb to 4762028 Compare July 28, 2025 19:37
@severinbeauvais
Copy link
Collaborator

@argush3 , @TVWerdal , @vysakh-menon-aot , could you please review this PR?

From Lucas:
image

Copy link
Collaborator

@TVWerdal TVWerdal left a comment

Choose a reason for hiding this comment

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

LGTM

@severinbeauvais severinbeauvais merged commit a02549f into bcgov:main Jul 28, 2025
4 checks passed
@loneil loneil deleted the feature/businessDigitalCredentialsQueue branch July 30, 2025 19:24
@severinbeauvais severinbeauvais changed the title Business Digital Credentials GCP queue service 29610 Business Digital Credentials GCP queue service Aug 5, 2025
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.

4 participants