Skip to content

use postgres and EF migrations#8

Open
Fildrance wants to merge 29 commits into
space-wizards:masterfrom
Fildrance:feature/use-fluent-migrator
Open

use postgres and EF migrations#8
Fildrance wants to merge 29 commits into
space-wizards:masterfrom
Fildrance:feature/use-fluent-migrator

Conversation

@Fildrance

Copy link
Copy Markdown
Contributor

No description provided.

@Fildrance

Copy link
Copy Markdown
Contributor Author

Needs integration test for repository.

@Fildrance

Copy link
Copy Markdown
Contributor Author

added test

Comment thread docker-compose-debug.yml
@@ -0,0 +1,12 @@
services:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change would mean that using docker compose up wouldn't actually start the labeller. It would only start a local postgres db. The docker compose is to be used in prod as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changed it back

Comment thread docker-compose.yml Outdated
Comment thread SS14.Labeller/Repository/RepositoryBase.cs Outdated
Comment thread SS14.Labeller/Registry.cs Outdated
Comment thread SS14.Labeller/Registry.cs Outdated
Comment thread SS14.Labeller/Registry.cs Outdated
Comment thread SS14.Labeller/Database/Migrations/001_CreateTable_DiscourseTopics.cs Outdated
Comment thread SS14.Labeller/Database/DatabaseMigration.cs Outdated
Comment thread README.md Outdated
Comment on lines +122 to +128
### Debugging behaviour in container

To debug app behaviour in container environment you can use docker-compose-debug.yaml (it will pick latest version of labeller app from image repository):
```
docker compose -f docker-compose-debug.yml up -d
```
Or build Dockerfile yourself to try out your local code. No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Feels kinda sus, unsure. Shouldn't local docker compose files not use the images from the registry? You would want to test your own changes, no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was under impression that prod is not using docker-compose, i guess its better to discuss what do we exactly want. I added this one because its the fastest way to start infrastructure locally - you just go
docker compose up
and you then start application locally for debug - very nice stuff. If we use docker-compose on prod - it should contain application, but i am not 100% sure how we do that... and i am also not sure its a good idea to use docker-provided postgres - we need to think ahead, if it will contain data, loss of which will be major PITA to project as a whole. We probably would need good postgres instance with backups, metrics and alerts for jetfish - why not let it live with that thing and think about it a little bit ahead of time?

Comment thread .github/workflows/build-test.yml Outdated
Comment thread SS14.Labeller.sln
Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 17
VisualStudioVersion = 17.13.35818.85 d17.13
VisualStudioVersion = 17.13.35818.85

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

pepeHands when will M$ release xml-based sln files, ffs

@Fildrance Fildrance requested a review from Simyon264 September 8, 2025 18:31
Comment thread SS14.Labeller/appsettings.json Outdated
"ConnectionStrings":{
"Default": "Data Source=data/Application.db"
"ConnectionStrings": {
"Default": "Host=localhost;Port=5432;Username=labeller_user;Password=example;Database=labeller;"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Usually for your local postgres instances the password is "postgres" and the user is also "postgres", change it to this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changed.

Comment thread Dockerfile
"linux/arm64") export RID=linux-arm64 ;; \
*) echo "Unsupported TARGETPLATFORM: $TARGETPLATFORM" && exit 1 ;; \
esac \
&& dotnet publish -c Release -r $RID --self-contained true /p:PublishAot=true -o /app

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This would make the container not run since the dotnet runtime is not in the container

@Fildrance Fildrance Sep 29, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

--self-contained true is still there - why would there be problem with that?!
Tested it with this Dockerfile and got no 'missing dotnet' problem :/ am i doing something wrong?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh right! Yeah. Though for download size it would probably be best that the Docker image your standard dotnet runtime image since then users won't have to download the runtime twice if they already had a docker image with it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So you would better like to have 9.0.9-alpine3.22 here instead of what it currently uses?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yea, something like that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@Fildrance Fildrance changed the title Feature/use fluent migrator use postgres and EF migrations Sep 29, 2025
@Fildrance Fildrance requested a review from Simyon264 September 30, 2025 06:53
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.

2 participants