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

Local containers #248

Merged
merged 7 commits into from
Dec 3, 2024
Merged

Local containers #248

merged 7 commits into from
Dec 3, 2024

Conversation

mpeels
Copy link
Collaborator

@mpeels mpeels commented Dec 2, 2024

Newly added

  1. Updates the docker compose to use the existing dataingestion.env
  2. Updates the README with newly required fields
    initial
    keycloak

Notes - Previously reviewed

  1. Adds a docker container for the NBS databases.
  2. Adds a docker container for a local keycloak instance to simplify local development
  3. Adds support for an application-local.yaml for the data-ingestion-service.
  4. Updates gradle from 8.0 to 8.10.1

Checklist

Types of changes

What types of changes does this PR introduces?

  • Bugfix
  • New feature
  • Breaking change
  • Local dev improvements

Testing

  • Does this PR has >90% code coverage?
  • Is the screenshot attached for code coverage?
  • Does the gradle build pass in your local?
  • Is the gradle build logs attached?

image

@mpeels mpeels requested a review from ndduc01 December 2, 2024 20:52
@mpeels mpeels requested a review from ssathiah December 2, 2024 20:58
Copy link
Collaborator

@ssathiah ssathiah left a comment

Choose a reason for hiding this comment

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

Looks good.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does dataingestion.env require any new variables?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

By default the docker-compose.yml will read any .env file. So currently I have the DATABASE_PASSWORD and KEYCLOAK_ADMIN_PASSWORD specified in it.

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 don't like having two .env files here. I'll consolidate my new .env into the existing dataingestion.env and update the README

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks.

@mpeels mpeels requested a review from snehaasjournal December 2, 2024 22:30
Copy link

sonarqubecloud bot commented Dec 2, 2024

@mpeels mpeels merged commit bb7f942 into main Dec 3, 2024
3 checks passed
@mpeels mpeels deleted the local-containers branch December 3, 2024 15:05
snehaasjournal pushed a commit that referenced this pull request Dec 31, 2024
* Ignore application-local

* Add containers directory

* Update docker compose

* Add local profile to bootRun

* Health check for db

* Update gradle

* Update README, use dataingestion.env for db and keycloak

---------

Co-authored-by: Michael Peels <[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.

3 participants