EotvosCollegium / mars

Unified IT System of the Eötvös József College for Advanced Studies.
https://uran.eotvos.elte.hu
MIT License
2 stars 15 forks source link

Deploying recent application-related changes #610

Closed viktorcsimma closed 3 months ago

viktorcsimma commented 3 months ago

Merges the contents of #595, #601, #602 and #606 into deployment.

BertalanD commented 3 months ago

I think we should go through the usual process of testing this on staging first.

I'd also like to run a backup of the prod database before deploying these changes. The chances of things going wrong are pretty slim; but still, we need to run a migration, and there is already live applicant data on the system. It would be a disaster to lose it.

viktorcsimma commented 3 months ago

You are right. How should we do this? I'd be glad if you could do it, but I can also click through it.

Also, it has crossed my mind whether we need the staging branch for testing. Maybe it would be simpler to simply run the staging system from the development branch; that way, we only need one PR instead of two. WDYT?

BertalanD commented 3 months ago

How should we do this? I'd be glad if you could do it, but I can also click through it.

I think the easiest would be if I pushed up the merge commit to the staging branch. I've tested these commits locally, but it would still be interesting to check if everything works well with the production data.

Maybe it would be simpler to simply run the staging system from the development branch

I think this could work. However, this setup was @horcsinbalint's idea, so I'd love to hear his take on this.

One counterargument I could imagine is that staging contains more trustworthy code, as it ideally goes through one more round of review when development is merged into staging. Maybe if more people had permissions to merge things into development than staging, this might even make sense.

This also hasn't happened before: but if we wanted to keep the commit history clean, we could have a policy of reverting staging to a working state, and fixing development "forward" instead.

horcsinbalint commented 3 months ago

Also, it has crossed my mind whether we need the staging branch for testing. Maybe it would be simpler to simply run the staging system from the development branch; that way, we only need one PR instead of two. WDYT?

There have been a discussion on it in #556 as well

viktorcsimma commented 3 months ago

I think the easiest would be if I pushed up the merge commit to the staging branch. I've tested these commits locally, but it would still be interesting to check if everything works well with the production data.

That's fine:) could you try it?

One counterargument I could imagine is that staging contains more trustworthy code, as it ideally goes through one more round of review when development is merged into staging. Maybe if more people had permissions to merge things into development than staging, this might even make sense.

We should then define what qualifies for being merged into staging from development; do you have any ideas for this? Also, we would need a mechanism for bug fixes that have to be deployed urgently.

But I think it is easier to simply use only one step.

viktorcsimma commented 3 months ago

@horcsinbalint now I see what you've written in #556... what if we retained the staging branch but opened no separate PRs for it? That way, we could use that tag flexibly and test/try things, while still keeping the development branch clean. (Basically, this is what we are planning to do right now.)

horcsinbalint commented 3 months ago

@horcsinbalint now I see what you've written in #556... what if we retained the staging branch but opened no separate PRs for it? That way, we could use that tag flexibly and test/try things, while still keeping the development branch clean.

I stopped creating PRs for them except for cases where I did not agree with the contents of the changes:

viktorcsimma commented 3 months ago

I see; it’s fine for me then:) what annoyed me was the two-stage PR process. But if we don’t do separate PRs for staging, it is fine.

BertalanD commented 3 months ago

The backup is done. Everything looks okay according to my testing. will push to deployment manually.