AttributeError: 'ModelInfo' object has no attribute 'securityStatus'

[Wauplin]

Describe the bug

Occasionally, a test fails in CI with the error AttributeError: 'ModelInfo' object has no attribute 'securityStatus'.
It also happened to me once on my local machine and I was able debug it a bit more. The error came from the fact that the model info was returned by the server without a securityStatus field, even though securityStatus is set to True in the test (see log below).

2 unrelated things here:

1. Some tests are occasionally failing because of a server error while running the tests. It seems that test stability has been successfully addressed in Improve tests and logging #682. However, the retry_endpoint decorator cannot work here because it's not a classic HTTPError but a more pernicious error where the server returns something but not with the expected content. Any idea what could cause that ?

2. When securityStatus is not returned by the server, ModelInfo does not have the attribute at all because of the following hack:

   # in ModelInfo.__init__()
   # (...)
   for k, v in kwargs.items():
       setattr(self, k, v)

   I remember that I read somewhere this is because we want older versions of huggingface_hub to still contain future informations, which is a good reason for having it.
   However, since this attribute is now expected and even tested (see this test), I would be favorable to add it as a permanent attribute to ModelInfo (with a default value to None for example).

Reproduction

Happens occasionally. See https://github.com/huggingface/huggingface_hub/runs/7918045010?check_suite_focus=true for example.

Logs

self = <tests.test_hf_api.HfApiPublicTest testMethod=test_model_info_with_security>

    @with_production_testing
    def test_model_info_with_security(self):
        _api = HfApi()
        model = _api.model_info(
            repo_id=DUMMY_MODEL_ID,
            revision=DUMMY_MODEL_ID_REVISION_ONE_SPECIFIC_COMMIT,
            securityStatus=True,
        )
>       self.assertEqual(model.securityStatus, {"containsInfected": False})
E       AttributeError: 'ModelInfo' object has no attribute 'securityStatus'

System Info

Github CI (commit 3ce9e18129f690be1c528c9cd1d13bc37a58aab5 for example)

[osanseviero]

Iirc, we've seen this in the past because the staging Hub does not have security status on it, which leads to the issues you mention.

cc @McPatate @SBrandeis

[Wauplin]

@osanseviero ok, good to know.
In this case the test is using @with_production_testing decorator which should prevent this error from happening right ? It is possible that in some cases the call is still made/redirected to the staging endpoint ?

[Wauplin]

I isolated the issue and reported it to moon-landing (private link). Doesn't seem to be related to huggingface_hub itself. For the time being, let's just ignore the failing test :/

[LysandreJik]

In that situation, we can likely just skip the failing test so that we still have green ticks on new PRs until the issue is resolved and keep this issue open to track it; imo this is especially important to continue having (alarming) red ticks otherwise, so that we pay attention to what might be failing rather than trusting it's the test that always fails.

WDYT @Wauplin?

[Wauplin]

Makes total sense @LysandreJik ! I'll take care of it :)

My "let's just ignore the failing test :/" seems so lazy now that I read it again 😁

[Wauplin]

@LysandreJik I just created the PR to skip the test.

Not related to the bug but mentioned in my first comment, do you have an opinion about setting securityStatus as a permanent attribute of ModelInfo ? In any case, if I change that it will be later in a separate PR.

[LysandreJik]

Adding it with a default value of None sounds good to me. There was a discussion about a similar attribute here which seems to echo what you're proposing, so all good for me.