Is SQLAlchemy 2.0 supported?

[chkpnt]

Hello Ralph,

Thank you very much for developing and maintaining fangfrisch, which I want to use to replace clamav-unofficial-sigs. 🙂

I must admit, I'm not really familiar with Python, but I'm currently trying to build an rpm for openSUSE. In the requirements.txt, you write "SQLAlchemy >= 1.4.0". Does this include SQLAlchemy 2.0, or should I stick with a version 1.4.x? I'm asking because, coming from the Java world, I know that with OR mappers, a major update often isn't straightforward, and you might have set the requirement at a time when v2 wasn't even released yet.

Best regards,
Gregor

[rseichter]

Hello Gregor. Short answer: SQLAlachemy 2 is fine.

If you check the Actions section of this repository, you will find that the unit tests are run for each Python version from 3.7 to 3.12, and that the dependencies installed during the build's test phase include SQLAlchemy 2 (see here for example). Also, at the time of writing this, my own production servers use SQLAlchemy 2.0.28, and they have been using various versions 2.0.x over the years following Gentoo Linux's history of the dev-python/sqlalchemy package. I never had any problems.

Speaking of, should you encounter Python-related problems creating the openSUSE package (which would be a welcome addition, thank you for considering it), please let me know. Perhaps I can help.

[chkpnt]

Thanks for your insights!

Now here is my package: https://build.opensuse.org/package/show/home:chkpnt:mailserver/python-fangfrisch

I'm open to any feedback on the package. I've packaged it in a way, so that just the timer needs to be enabled (systemctl enable fangfrisch.timer) and started (systemctl start fangfrisch.timer), hence the wrapper script run-fangfrisch.sh.

But please do not link to it just yet. I need to determine how to move it to devel:languages:python, as there is already a package there, that isn't well-maintained; otherwise, I would have used that one.

[rseichter]

I am unfamiliar with openSUSE packing, so please take my comments with a grain of salt.

At first glance, you seem to have added some complexity which is, or at least could be, unnecessary. For example, checking if the configuration file exists is something that Fangfrisch does anyway. Other aspects of your wrapper script could be handled by fangfrisch.{service,timer}. The Archlinux package can offer some ideas. Lastly, patching upstream files is at best an invitation to extra work in the future. Anything not explicitly defined as stable/API is prone to unannounced changes.

All this is meant as feedback, not criticism. I am grateful for your work.

[chkpnt]

Yes, the arch package is using ConditionPathExists=/var/lib/fangfrisch/db.sqlite in its unit file to prevent the start if the database isn't initialized yet. But in my opinion, this is error prone:

• If the location is changed in /etc/fangfrisch.conf, the unit-file has to be adapted, too. But a package update will reset /usr/lib/systemd/system/fangfrisch.service, so the admin has to place a drop-in snippet in /etc/systemd/fangfrisch.conf.d/
• When the service can't be started due to a failing call of initdb, there is the risk that the admin will run fangfrisch under the wrong user (root). systemd should print a message "Please run sudo -u vscan -- fangfrisch --conf /etc/fangfrisch.conf initdb", but I don't see a way to do so. But if the system knows that initdb needs to be called, why not calling it directly?

What do you thing of an argument like --init-if-needed, so that refresh could be used without initdb in advance?

It's only the example script for handling Fangfrisch news, that I'm patching. Whereas the first patch is your own commit, which isn't included in 1.9.0. fangfrisch-has-news.sh is perfect for an example or an unpackaged script. 😀 I wouldn't write it different when placing the script in ~/bin/ or /usr/local/bin. But in a rpm packaged for openSUSE following the FHS, it should be located in /usr/libexec/fangfrisch/. And as the script contains configuration options, I would need to annotate it with %config(noreplace) in the spec-File, so that a package update doesn't replace the script (in such case, the new script would be placed as fangfrisch-has-news.sh.rpmnew next to the already existing file). But that is unpleasant, as the script might evolve. (I'm also getting a warning by the build system, when placing a config file in /usr/{libexec,lib}.)

[rseichter]

Like I said, I don't know about openSUSE packaging specifics, while you on the other hand have obviously thought carefully about this package. However, my point about you not needing to worry about a potentially missing configuration file stands. 😉 Fangfrisch is quite vocal when unable to parse the config file.

As for DB initialisation-on-demand: That would be nice, but also tricky, because it would require me interfering with SQLAlchemy. I don't want to meddle with the database abstraction layer, as you can surely understand.