Implement build number manager (Django) #2
Conversation
Hartmnt
force-pushed
the
v2_django
branch
6 times, most recently
from
e1c28f5 to
da8fead
Compare
Hartmnt
changed the title
|
Ready for review @Krzmbrzl |
There was a problem hiding this comment.
Alright, it seems that I would be happy to continue using Django for our purposes. It appears to offer a lot of functionality while still remaining quite flexible in terms of what can be done.
Type hints are a bit off from time to time, but we can deal with that once we have set up CI (which should contain a type-checker like pyright)
mumble_backend/build_number_generator/migrations/0001_initial.py
Outdated
Show resolved
Hide resolved
| # SECURITY WARNING: keep the secret key used in production secret! | ||
| SECRET_KEY = "django-insecure-0m)dqf_kuj3e0tp*3zwfyfm89l(fyn6!zyw5j1o2b3m5j)r2bq" | ||
|
|
||
| # SECURITY WARNING: don't run with debug turned on in production! | ||
| DEBUG = True |
There was a problem hiding this comment.
I'm wondering whether there might be the risk of forgetting to change these in production. Is there a way to make it error if those are not changed?
| AUTHORIZED_HASHES = { | ||
| # 'testpassword' | ||
| "ebe5a79f942bab98b3c122ceb72a9c23fb21991848cddb9ad4ce8a209c8269c25eb5f5146427afc23ef588de61798b38451282339e2c637c6dec51d635ab50c4" | ||
| } |
There was a problem hiding this comment.
How can we avoid this from accidentally slipping into production?
Hartmnt
force-pushed
the
v2_django
branch
3 times, most recently
from
87f58e0 to
db41ac2
Compare
Hartmnt
changed the title
Hartmnt
force-pushed
the
v2_django
branch
3 times, most recently
from
af4eba4 to
279858c
Compare
|
@Krzmbrzl With the new requirements i have re-structured the entire app. For an overview of the API endpoints check out https://github.com/Hartmnt/mumble-backend-services/blob/v2_django/mumble_backend/build_number_manager/README.md I have yet to figure out the going-into-production part |
|
Based on the API description: For the current workflow with build numbers we need the GET request to implicitly create a new build number if none exists yet. So we don't use specific requests to create new build numbers. We simply have our CI runners fetch the build number and if no such build number exists yet, the backend is expected to create that build number and return that (provided that the GET request authenticated with the proper token - if the token doesn't match or there is no token, then this implicit creation must not happen and an error must be returned instead, if the requested build number doesn't exist yet). |
|
Though maybe it would be cleaner to have these steps as separate requests. I guess we could also adapt the Python script that does the fetching to make different requests for different purposes 🤔 |
|
@Krzmbrzl So, the first of my implementations was designed to do all the steps in one go as you describe above. However, if we want more functionality (for example displaying a list of commits) I would strongly suggest adapting a REST-like architecture. That means for example no implicit data creation especially not on GET requests. It would indeed mean changing the deploy scripts to perform two separate requests. I would imagine this is not a big deal. The default expected result should be considered: For example, if we almost always expect to create a new build number when the pipeline runs, we would try to create a new build number with the API. And if and only if that fails with 409, we would then instead get the existing number. But idk, maybe the other case is more common and we first check if the build already exists - I have no overview over that. Either way, I think we should stick to REST-like and only perform "one" expected action per API call... |
No description provided.