New WaitingListEntry #33
No reviewers
Labels
No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: data.coop/membersystem#33
Loading…
Reference in a new issue
No description provided.
Delete branch "benjaoming/membersystem:waiting-list"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Sorted the pre-commit things... some were because of
src/static
being included, and some have been fixed in another PR 🎉27b9a99e93
to2317138111
2317138111
to1405a95094
Yeah, I didn't go through all the ruff-stuff. I've done so now in
f18469833a
53c72a9b46
tod769481848
WIP: New WaitingListEntry + bunch of pre-commit stuffto New WaitingListEntry + bunch of pre-commit stuffSeems 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.
Unsure why it's failing, is it because I'm building from my own repo?
Anyways, everything works fine locally 👍
New WaitingListEntry + bunch of pre-commit stuffto New WaitingListEntry@ -27,0 +30,4 @@
test: [ "CMD-SHELL", "pg_isready -U postgres -d postgres" ]
interval: 5s
timeout: 5s
retries: 30
@valberg this is where I added the healthcheck 🤦
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.
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 :)
Reverted!
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 😊
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.
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.
@ -40,0 +40,4 @@
chown www:www /app/src/static
RUN pip install --no-cache-dir -r $REQUIREMENTS_FILE
RUN django-admin compilemessages
Breaking the single RUN statement into multiple statements means bigger image.
Yes but having to re-run the same things over and over again, creating huge new cache layers every time would be equally bad?
Or which RUN commands is it that you want to see merged here?
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 😆)@ -5,3 +5,1 @@
from .models import Membership
from .models import MembershipType
from .models import SubscriptionPeriod
from . import models
I'm against importing whole modules.
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.
Well, yes. I mostly use for instance
from django.db import models
in my ownmodels
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".
I also use
from django.db import models
inapp.models
but inapp.*
I usefrom 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 othermodels
module, then something is wrong.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)
Alright you stubborn one ❤️
Nice! I just have two things.
981fde1322
to801f9fd209
🎉