Local development environment refresh and update

jgroman commented

2025-11-28 15:59:54 +00:00

Owner

added local SQLite support to alembic DB upgrade scripts
updated flask-oidc library version to the latest version available to fix potential security problem and also to enable mocking OIDC users for local development
updated README with notes regarding local development setup

- added local SQLite support to alembic DB upgrade scripts - updated flask-oidc library version to the latest version available to fix potential security problem and also to enable mocking OIDC users for local development - updated README with notes regarding local development setup

kparal commented

2025-12-01 10:24:35 +00:00

Owner

It would be good to standardize on PostgreSQL only, to avoid incompatibilities. See documentation in blockerbugs, hopefully it can help: https://pagure.io/fedora-qa/blockerbugs/blob/develop/f/docs/source
Thanks!

It would be good to standardize on PostgreSQL only, to avoid incompatibilities. See documentation in blockerbugs, hopefully it can help: https://pagure.io/fedora-qa/blockerbugs/blob/develop/f/docs/source Thanks!

jgroman commented

2025-12-01 16:21:37 +00:00

Author

Owner

1 new commit added

Change local dev db from SQLite to PostgreSQL

**1 new commit added** * ``Change local dev db from SQLite to PostgreSQL``

jgroman commented

2025-12-02 13:09:17 +00:00

Author

Owner

2 new commits added

Update app cli to take the current DB schema into account
Create Makefile for simplified app operations

**2 new commits added** * ``Update app cli to take the current DB schema into account`` * ``Create Makefile for simplified app operations``

jgroman commented

2025-12-03 08:07:48 +00:00

Author

Owner

Ready for review

kparal commented

2025-12-03 09:57:08 +00:00

Owner

Thanks, Jaroslav! My notes:

If you're happy writing and maintaining a Makefile, it works for me. I personally changed it to a shell script in blockerbugs, because Makefile syntax is very difficult and I saw no actual advantage in using it. Up to you.
I see that all the "make it sqlite compatible" alembic updates are still included, do you want to keep them? I personally don't care whether we commit the code or drop it, but just to clarify - I think we're not going to care about sqlite compatibility in the future, are we?
Thanks a lot for writing up the developer documentation!

I have a few more in-line comments coming.

After the review, I suggest the existing commits could be squashed into one or a few commits, before merging, so that the final commit history is nice to read.

Thanks, Jaroslav! My notes: 1. If you're happy writing and maintaining a Makefile, it works for me. I personally changed it to a [shell script](https://pagure.io/fedora-qa/blockerbugs/blob/develop/f/run) in blockerbugs, because Makefile syntax is very difficult and I saw no actual advantage in using it. Up to you. 2. I see that all the "make it sqlite compatible" alembic updates are still included, do you want to keep them? I personally don't care whether we commit the code or drop it, but just to clarify - I think we're not going to care about sqlite compatibility in the future, are we? 3. Thanks a lot for writing up the developer documentation! I have a few more in-line comments coming. After the review, I suggest the existing commits could be squashed into one or a few commits, before merging, so that the final commit history is nice to read.

kparal commented

2025-12-03 10:02:59 +00:00

Owner

In blockerbugs we directed PGDATA to a non-default dir, so that it's stored directly inside the container. That means after starting the database again, your data aren't gone (if you want to purge the data, just remove the container and create it again, easy). And you also don't need to deal with mounted volumes, which requires more manual steps and can't be used out-of-the-box in the makefile.

I'm including that approach as a suggestion, we don't need to do it the same way here. I just wonder if you've seen that and have any opinion on which approach to take by default.

In [blockerbugs](https://pagure.io/fedora-qa/blockerbugs/blob/develop/f/docs/source/development.rst) we directed PGDATA to a non-default dir, so that it's stored directly inside the container. That means after starting the database again, your data aren't gone (if you want to purge the data, just remove the container and create it again, easy). And you also don't need to deal with mounted volumes, which requires more manual steps and can't be used out-of-the-box in the makefile. I'm including that approach as a suggestion, we don't need to do it the same way here. I just wonder if you've seen that and have any opinion on which approach to take by default.

kparal commented

2025-12-03 10:05:07 +00:00

Owner

Do you have any comparison of using virtualenv vs python -m venv? The former requires python3-virtualenv rpm to be installed, I believe. I'm not sure if there's any meaningful functional difference (in our case), so we might just go with the Python standard library?

Do you have any comparison of using `virtualenv` vs `python -m venv`? The former requires `python3-virtualenv` rpm to be installed, I believe. I'm not sure if there's any meaningful functional difference (in our case), so we might just go with the Python standard library?

kparal commented

2025-12-03 10:06:36 +00:00

Owner

That's all from me, thanks :)

jgroman commented

2025-12-03 10:15:23 +00:00

Author

Owner

If you're happy writing and maintaining a Makefile, it works for me. I personally changed it to a shell script in blockerbugs, because Makefile syntax is very difficult and I saw no actual advantage in using it. Up to you.

It is true that Makefile syntax might not be very readable at times. I am sort of used to use it from previous projects. There is one advantage in using this approach that it would sort out task dependencies for you so it is easy to make one target dependent on some other target without any additional boilerplate code.

I see that all the "make it sqlite compatible" alembic updates are still included, do you want to keep them? I personally don't care whether we commit the code or drop it, but just to clarify - I think we're not going to care about sqlite compatibility in the future, are we?

I think it might be useful to keep SQLite related updates in there if only just for future reference on how to workaround some PostgreSQL unique features like ALTER statement.
But SQLite compatibility will not be taken into account in future updates for sure.

After the review, I suggest the existing commits could be squashed into one or a few commits, before merging, so that the final commit history is nice to read.

OK, will do!

> 1. If you're happy writing and maintaining a Makefile, it works for me. I personally changed it to a [shell script](https://pagure.io/fedora-qa/blockerbugs/blob/develop/f/run) in blockerbugs, because Makefile syntax is very difficult and I saw no actual advantage in using it. Up to you. It is true that `Makefile` syntax might not be very readable at times. I am sort of used to use it from previous projects. There is one advantage in using this approach that it would sort out task dependencies for you so it is easy to make one target dependent on some other target without any additional boilerplate code. > 2. I see that all the "make it sqlite compatible" alembic updates are still included, do you want to keep them? I personally don't care whether we commit the code or drop it, but just to clarify - I think we're not going to care about sqlite compatibility in the future, are we? I think it might be useful to keep SQLite related updates in there if only just for future reference on how to workaround some PostgreSQL unique features like ALTER statement. But SQLite compatibility will not be taken into account in future updates for sure. > After the review, I suggest the existing commits could be squashed into one or a few commits, before merging, so that the final commit history is nice to read. OK, will do!

jgroman commented

2025-12-03 10:36:21 +00:00

Author

Owner

1 new commit added

Fix incorrect description of db data retention in container

**1 new commit added** * ``Fix incorrect description of db data retention in container``

jgroman commented

2025-12-03 10:37:36 +00:00

Author

Owner

In blockerbugs we directed PGDATA to a non-default dir, so that it's stored directly inside the container. That means after starting the database again, your data aren't gone (if you want to purge the data, just remove the container and create it again, easy). And you also don't need to deal with mounted volumes, which requires more manual steps and can't be used out-of-the-box in the makefile.

I have used a slightly different Postgres image from quay.io to remove dependency on Docker Hub. It allows for very similar workflow as you are mentioning, just without explicit setting of PGDATA. If no external volume is specified, the db will be stored inside the container just like in your example.
I have fixed incorrect description of this feature in DEVELOPER.md.

> In [blockerbugs](https://pagure.io/fedora-qa/blockerbugs/blob/develop/f/docs/source/development.rst) we directed PGDATA to a non-default dir, so that it's stored directly inside the container. That means after starting the database again, your data aren't gone (if you want to purge the data, just remove the container and create it again, easy). And you also don't need to deal with mounted volumes, which requires more manual steps and can't be used out-of-the-box in the makefile. I have used a slightly different Postgres image from quay.io to remove dependency on Docker Hub. It allows for very similar workflow as you are mentioning, just without explicit setting of PGDATA. If no external volume is specified, the db will be stored inside the container just like in your example. I have fixed incorrect description of this feature in DEVELOPER.md.

jgroman commented

2025-12-03 10:49:59 +00:00

Author

Owner

Do you have any comparison of using virtualenv vs python -m venv? The former requires python3-virtualenv rpm to be installed, I believe. I'm not sure if there's any meaningful functional difference (in our case), so we might just go with the Python standard library?

virtualenv developers claim it is superior to venv module: https://virtualenv.pypa.io/en/latest/
But you are right that those improvements are probably not very relevant in our use case and venv could be used as well. I'll update developer documentation as such.

> Do you have any comparison of using `virtualenv` vs `python -m venv`? The former requires `python3-virtualenv` rpm to be installed, I believe. I'm not sure if there's any meaningful functional difference (in our case), so we might just go with the Python standard library? `virtualenv` developers claim it is superior to `venv` module: https://virtualenv.pypa.io/en/latest/ But you are right that those improvements are probably not very relevant in our use case and `venv` could be used as well. I'll update developer documentation as such.

jgroman commented

2025-12-03 11:34:44 +00:00

Author

Owner

1 new commit added

Update python virtualenv docs in DEVELOPER.md

**1 new commit added** * ``Update python virtualenv docs in DEVELOPER.md``

kparal commented

2025-12-03 14:08:45 +00:00

Owner

Awesome, LGTM. Thanks a lot for starting with the basics and creating proper documentation and setup scripts before hacking on the code. This will make make our project much more approachable.

jgroman commented

2025-12-03 15:23:35 +00:00

Author

Owner

rebased onto f78fa1df18

rebased onto f78fa1df181f594951355dee3c680d6d7f36387f

jgroman commented

2025-12-03 15:29:43 +00:00

Author

Owner

Pull-Request has been merged by jgroman

Rows
Columns

Local development environment refresh and update #70