New WaitingListEntry #33

Merged
valberg merged 10 commits from benjaoming/membersystem:waiting-list into main 2024-07-31 22:49:48 +00:00
Owner

Sorted the pre-commit things... some were because of src/static being included, and some have been fixed in another PR 🎉

Sorted the pre-commit things... some were because of `src/static` being included, and some have been fixed in another PR 🎉
benjaoming added 1 commit 2024-07-14 16:25:04 +00:00
benjaoming force-pushed waiting-list from 27b9a99e93 to 2317138111 2024-07-14 16:59:39 +00:00 Compare
benjaoming force-pushed waiting-list from 2317138111 to 1405a95094 2024-07-14 17:20:38 +00:00 Compare
Owner

Yeah, I didn't go through all the ruff-stuff. I've done so now in f18469833a

Yeah, I didn't go through all the ruff-stuff. I've done so now in https://git.data.coop/data.coop/membersystem/commit/f18469833a6e5361281375ed8d6920a1d8389a34
benjaoming added 2 commits 2024-07-20 20:25:05 +00:00
benjaoming added 1 commit 2024-07-20 20:33:44 +00:00
Add a few more README things on Docker
Some checks failed
continuous-integration/drone/pr Build is failing
53c72a9b46
benjaoming force-pushed waiting-list from 53c72a9b46 to d769481848 2024-07-20 20:34:25 +00:00 Compare
benjaoming added 1 commit 2024-07-20 20:46:19 +00:00
Fix some issues after merge conflixts
Some checks failed
continuous-integration/drone/pr Build is failing
250f203900
benjaoming changed title from WIP: New WaitingListEntry + bunch of pre-commit stuff to New WaitingListEntry + bunch of pre-commit stuff 2024-07-20 20:49:44 +00:00
benjaoming requested review from valberg 2024-07-20 20:49:51 +00:00
Author
Owner

Seems ready now... there's some datefields as in all other models, so we can GDPR-clean our waiting list later, for instance to avoid storing emails indefinitely. I guess we'll invite people and then mark them "invited" or something.

Seems ready now... there's some datefields as in all other models, so we can GDPR-clean our waiting list later, for instance to avoid storing emails indefinitely. I guess we'll invite people and then mark them "invited" or something.
Author
Owner

Unsure why it's failing, is it because I'm building from my own repo?

Unsure why it's failing, is it because I'm building from my own repo?
Author
Owner

Anyways, everything works fine locally 👍

Anyways, everything works fine locally 👍
benjaoming changed title from New WaitingListEntry + bunch of pre-commit stuff to New WaitingListEntry 2024-07-24 09:37:56 +00:00
benjaoming reviewed 2024-07-24 12:05:11 +00:00
@ -27,0 +30,4 @@
test: [ "CMD-SHELL", "pg_isready -U postgres -d postgres" ]
interval: 5s
timeout: 5s
retries: 30
Author
Owner

@valberg this is where I added the healthcheck 🤦

@valberg this is where I added the healthcheck 🤦
Owner

Roger - the thing is that this compose file is for development, whereas the one we use for production is placed in https://git.data.coop/data.coop/ansible/src/branch/main/roles/docker/templates/compose-files/membersystem.yml.j2

With the current entrypoint one does not have to keep this health check in mind.

Roger - the thing is that this compose file is for development, whereas the one we use for production is placed in https://git.data.coop/data.coop/ansible/src/branch/main/roles/docker/templates/compose-files/membersystem.yml.j2 With the current entrypoint one does not have to keep this health check in mind.
Author
Owner

Aaaah, I get that now. This was intended to simplify things, but of course that's not the case when the deployment could benefit from the other method 👍 thanks for explaining! I took this method from a setup where Docker Compose was also for deployment :)

Aaaah, I get that now. This was intended to simplify things, but of course that's not the case when the deployment could benefit from the other method 👍 thanks for explaining! I took this method from a setup where Docker Compose was also for deployment :)
Author
Owner

Reverted!

Reverted!
Owner

You're welcome! We could put in the work to make it work for both. But since the membersystem is pretty much a single deployment project, I'm unsure of the benefit of doing said work 😊

You're welcome! We could put in the work to make it work for both. But since the membersystem is pretty much a single deployment project, I'm unsure of the benefit of doing said work 😊
Author
Owner

I think the main priority should be that all our data.coop services are somehow deployed in the same/similar way... but then their development environment can be allowed to drift.

Reasoning would be that we could have many people doing deployment/ops, and they should be required minimal familiarity with each project that we deploy?

So in this case, I think your choice was 💯 - i.e. make entrypoint.sh fit with the deployment, then let aspects of development setup come secondary.

I think the main priority should be that all our data.coop services are somehow deployed in the same/similar way... but then their development environment can be allowed to drift. Reasoning would be that we could have many people doing deployment/ops, and they should be required minimal familiarity with each project that we deploy? So in this case, I think your choice was 💯 - i.e. make entrypoint.sh fit with the deployment, then let aspects of development setup come secondary.
benjaoming marked this conversation as resolved
benjaoming added 1 commit 2024-07-24 20:29:40 +00:00
Revert healtcheck on Postgres, do it in the entrypoint
Some checks failed
continuous-integration/drone/pr Build is failing
fa6a5cdb86
Author
Owner

This is ready btw... I intend that we just use the Waiting List from the Django admin for now... then we can add a public interface later... BUT:

We have intentions that overlap heavily around "applying for membership" or "requesting an invitation", which I think we can probably integrate.

This is ready btw... I intend that we just use the Waiting List from the Django admin for now... then we can add a public interface later... BUT: We have intentions that overlap heavily around "applying for membership" or "requesting an invitation", which I think we can probably integrate.
benjaoming added 1 commit 2024-07-28 07:33:39 +00:00
Merge branch 'main' into waiting-list
Some checks failed
continuous-integration/drone/pr Build is failing
dd33a90896
valberg reviewed 2024-07-30 13:18:56 +00:00
Dockerfile Outdated
@ -40,0 +40,4 @@
chown www:www /app/src/static
RUN pip install --no-cache-dir -r $REQUIREMENTS_FILE
RUN django-admin compilemessages
Owner

Breaking the single RUN statement into multiple statements means bigger image.

Breaking the single RUN statement into multiple statements means bigger image.
Author
Owner

Yes but having to re-run the same things over and over again, creating huge new cache layers every time would be equally bad?

Yes but having to re-run the same things over and over again, creating huge new cache layers every time would be equally bad?
Author
Owner

Or which RUN commands is it that you want to see merged here?

Or which RUN commands is it that you want to see merged here?
Owner

So apparently the difference is a meager 6MB. The only thing we want is to have the requirements file when installing dependencies and then copy stuff in. I've done that in 0cf579c5f6 (the comical thing is that upgrading to bookworm at the same time has bumped the image size from 583MB to 681MB 😆)

So apparently the difference is a meager 6MB. The only thing we want is to have the requirements file when installing dependencies and then copy stuff in. I've done that in https://git.data.coop/data.coop/membersystem/commit/0cf579c5f6e70b837c487e254532987d7186a56f (the comical thing is that upgrading to bookworm at the same time has bumped the image size from 583MB to 681MB 😆)
valberg reviewed 2024-07-31 21:52:45 +00:00
@ -5,3 +5,1 @@
from .models import Membership
from .models import MembershipType
from .models import SubscriptionPeriod
from . import models
Owner

I'm against importing whole modules.

Explicit is better than implicit.

I'm against importing whole modules. > Explicit is better than implicit.
Author
Owner

Don't we import whole modules everywhere all the time?

The idea is that the Django app is a domain-specific app so importing the models into the admin makes sense because almost all Models from that models module will have an admin. To me, this makes the code more readable than having lots of import statements.

Don't we import whole modules everywhere all the time? The idea is that the Django app is a domain-specific app so importing the models into the admin makes sense because almost all Models from that models module will have an admin. To me, this makes the code more readable than having lots of import statements.
Owner

Well, yes. I mostly use for instance from django.db import models in my own models modules, because it is a standard - but I actually would love to be more explicit everywhere.

When push comes to shove it is all about preference. My preference is to explicitly import what is being used, to have a clear "in this module we are using these external things".

Well, yes. I mostly use for instance `from django.db import models` in my own `models` modules, because it is a standard - but I actually would love to be more explicit everywhere. When push comes to shove it is all about preference. My preference is to explicitly import what is being used, to have a clear "in this module we are using these external things".
Author
Owner

I also use from django.db import models in app.models but in app.* I use from app import models because it's so convenient and it targets the domain-specific ideal of the app. As in, if you are import some other models module, then something is wrong.

I also use `from django.db import models` in `app.models` but in `app.*` I use `from app import models` because it's so convenient and it targets the domain-specific ideal of the app. As in, if you are import some other `models` module, then something is wrong.
Owner

I think having explicit imports is more readable and gives me an overview of what the module is interfacing with.

I think we can debate this forever :P

I would say that we should go with "do not change it just for the sake of changing it" and keep it as it was (but that is also in favorable for me ATM)

I think having explicit imports is more readable and gives me an overview of what the module is interfacing with. I think we can debate this forever :P I would say that we should go with "do not change it just for the sake of changing it" and keep it as it was (but that is also in favorable for me ATM)
Author
Owner

Alright you stubborn one ❤️

Alright you stubborn one ❤️
benjaoming marked this conversation as resolved
valberg requested changes 2024-07-31 21:56:33 +00:00
Dismissed
valberg left a comment
Owner

Nice! I just have two things.

Nice! I just have two things.
benjaoming added 2 commits 2024-07-31 22:44:14 +00:00
benjaoming added 1 commit 2024-07-31 22:46:21 +00:00
Merge branch 'main' of git.data.coop:data.coop/membersystem into waiting-list
Some checks failed
continuous-integration/drone/pr Build is failing
981fde1322
benjaoming force-pushed waiting-list from 981fde1322 to 801f9fd209 2024-07-31 22:47:40 +00:00 Compare
benjaoming requested review from valberg 2024-07-31 22:48:39 +00:00
valberg approved these changes 2024-07-31 22:49:38 +00:00
valberg merged commit 865bc6c7bd into main 2024-07-31 22:49:48 +00:00
valberg referenced this pull request from a commit 2024-07-31 22:49:52 +00:00
valberg deleted branch waiting-list 2024-07-31 22:50:42 +00:00
Author
Owner

🎉

🎉
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: data.coop/membersystem#33
No description provided.