audeering / audbackend

Manage file storage on different backends
https://audeering.github.io/audbackend/
Other
3 stars 0 forks source link

Base._ls(): may always return an empty list #178

Closed frankenjoe closed 9 months ago

frankenjoe commented 9 months ago

So far the docstring of Base._ls() was saying:

        If ``path`` is ``'/'`` and no files exist on the repository,
        an empty list should be returned
        Otherwise,
        if ``path`` does not exist or no files are found under ``path``,
        an error should be raised.

This might be overlooked by a developer, so here we simplify _ls() by not demanding to raise an error message or treat / as a special case. Instead it is sufficient to return an empty list, even in the case that the path does not exist. The docstring now says:

        If ``path`` does not exist
        an empty list can be returned.

To avoid a breaking change, raising a file-not-found-error is now handled by Base, in the same way we already do it for the case that path is not a sub-path.

frankenjoe commented 9 months ago

@hagenw locally tests with Artifactory are passing, but here it seems that it can no longer access the server. Any idea?

hagenw commented 9 months ago

There is a different when running locally or here in the CI as the CI uses the audeering-unittest user, whereas we locally use our user names. But all of those users have exactly the same rights in the Artifactory server.

The error message of the failing test was:

E           dohq_artifactory.exception.ArtifactoryException: This request is blocked due to recurrent login failures, please try again in 7 seconds

My suspicision is that every time we try to access a file that does not exist it will be counted as a login failure. And as we have several CI jobs running in parallel, it might be that we reach the blocking threshold too early. To me it does not sound like a good design on the Artifactory side that a user gets block even she/he has a correct username/password login.

I guess we need to check if there is a setting on the Artifactory server to change the behavior. If not, we might have to reduce the number of pipelines. Or group tests for error messages on missing data in a separate file and run this file only in a single CI job.

hagenw commented 9 months ago

Maybe there is also an error with the stored secret on Github. I will try to replace it with a new one.

frankenjoe commented 9 months ago

But isn't it strange we only encounter this problem now?

frankenjoe commented 9 months ago

Maybe there is also an error with the stored secret on Github. I will try to replace it with a new one.

Ok, thanks.

hagenw commented 9 months ago

But isn't it strange we only encounter this problem now?

I also encountered the problem when I tried to add tests for non-existing and private repositories in https://github.com/audeering/audb/pull/339

hagenw commented 9 months ago

I replaced the secret on Github and re-started the CI jobs, but unfortunately the error stays the same.

hagenw commented 9 months ago

It seems to be common that secrets are replaced by "***" in the output of CI jobs. Here is the output of an older passing one:

image

hagenw commented 9 months ago

I tested a few more things and the username and API key seem still to be correctly set, but the error remains in the end:

E           dohq_artifactory.exception.ArtifactoryException: This request is blocked due to recurrent login failures, please try again in 13 seconds

Maybe we wait until next week to see if the error remains?

frankenjoe commented 9 months ago

Maybe we wait until next week to see if the error remains?

ok

codecov[bot] commented 9 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (65c6844) 100.0% compared to head (fa10a86) 100.0%.

:exclamation: Current head fa10a86 differs from pull request most recent head 22ab9e4. Consider uploading reports for the commit 22ab9e4 to get more accurate results

Additional details and impacted files | [Files](https://app.codecov.io/gh/audeering/audbackend/pull/178?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering) | Coverage Δ | | |---|---|---| | [audbackend/core/backend/artifactory.py](https://app.codecov.io/gh/audeering/audbackend/pull/178?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering#diff-YXVkYmFja2VuZC9jb3JlL2JhY2tlbmQvYXJ0aWZhY3RvcnkucHk=) | `100.0% <100.0%> (ø)` | | | [audbackend/core/backend/base.py](https://app.codecov.io/gh/audeering/audbackend/pull/178?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering#diff-YXVkYmFja2VuZC9jb3JlL2JhY2tlbmQvYmFzZS5weQ==) | `100.0% <100.0%> (ø)` | | | [audbackend/core/backend/filesystem.py](https://app.codecov.io/gh/audeering/audbackend/pull/178?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=audeering#diff-YXVkYmFja2VuZC9jb3JlL2JhY2tlbmQvZmlsZXN5c3RlbS5weQ==) | `100.0% <100.0%> (ø)` | |
hagenw commented 9 months ago

As usual, waiting a few days helps to solve login issues.