Conversation
MikeNeilson
left a comment
MikeNeilson
left a comment
There was a problem hiding this comment.
Most of this looks reason. Some odd spelling changes though.
| "log" | ||
|
|
||
| "github.com/aws/aws-sdk-go/aws" | ||
| // "github.com/aws/aws-sdk-go/aws" |
There was a problem hiding this comment.
If not required, just delete.
| } | ||
|
|
||
| func (cfg Config) AWSConfig() aws.Config { | ||
| // func (cfg Config) AWSConfig() aws.Config { |
There was a problem hiding this comment.
same as above, if code is no longer required, just remove it.
| "github.com/jmoiron/sqlx" | ||
| "github.com/labstack/echo/v4" | ||
| // "github.com/jmoiron/sqlx" | ||
| // "github.com/labstack/echo/v4" |
There was a problem hiding this comment.
favor deletion over commenting out code.
| # Install Git | ||
| RUN apk update && apk add --no-cache ca-certificates | ||
|
|
||
| RUN apk update && \ |
There was a problem hiding this comment.
Only upgrade should be required, also consider splitting the upgrade and add steps. Can speed things up for local dev if you need to add a package.
See https://github.com/opendcs/opendcs/blob/8655b71b0040cc6ca7e18c6cdfb465f844e852ca/Dockerfile#L17-L18 (NOTE: also possible the update is required, in which case we should fix opendcs.)
| # S3 Configuration | ||
| # ------------------------- # | ||
| WRITE_TO_BUCKET: str = os.getenv("WRITE_TO_BUCKET", default="castle-data-develop") | ||
| WRITE_TO_BUCKET: str = os.getenv("WRITE_TO_BUCKET", default="caste-data-develop") |
There was a problem hiding this comment.
was the bucket named incorrectly? castle is an objectively correct spelling.
| depends_on: | ||
| cumulusdb: | ||
| condition: service_started No newline at end of file | ||
| condition: service_started |
There was a problem hiding this comment.
service_healthy if likely a better choice here.
3 participants
Go and aws sdk version bump, and docker clean up