Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow Classifer.classifier from being optional #16986

Open
miketheman opened this issue Oct 28, 2024 · 0 comments
Open

Disallow Classifer.classifier from being optional #16986

miketheman opened this issue Oct 28, 2024 · 0 comments

Comments

@miketheman
Copy link
Member

It's unlikely, but possible, that a Classifier.classifer with a None value is created and stored in the database.

from warehouse.classifiers.models import Classifier
c = Classifier(classifier=None)
db.add(c)
db.commit()

No error is raised.

There's no database constraint for this either, and Postgres will allow any number of NULL records to exist despite the unique constraint, since every NULL is treated as unique. Much special.

I think there's enough safety in the runtime system to not freak out if there's not a value there, but it's something that the database and model should reject. However the existence of a NULL in production would block the bin/release step where we sync the classifiers, with this kind of error:

...
  File "/opt/warehouse/src/warehouse/cli/classifiers.py", line 59, in sync
    classifier.ordering = sorted_classifiers.index(classifier.classifier)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: None is not in list

Since the sync command gets all of them, regardless of their values (which I believe to be correct for this use case).

This is probably a relatively small change with a migration - and in production the table is only 867 records long, and is rarely updated (other than during a deploy if needed), so is unlikely to take many locks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant