pagure

#5405 Fixes to build and run Pagure on Fedora 38
Closed by wombelix. Opened by klauskoder.
klauskoder/pagure build-fedora38  into  master

Download 5405.patch

Pagure is, as far as I can tell, completely broken on Fedora 38.

Installing the official Fedora version from dnf gives you the last released version of pagure, that is5.13.3 from 2021. This version crash left and right at
runtime. pagure-ev crash when started because of bit-rot in trololio.

Trying to build the packages using the description in the documentation fail because of a number of issues, it was definitely never tested.

The two patches here attempt to fix these two issues:

1) Commit 49df7760e : Update rpm-spec file and python requirements.txt
2) Commit fd4e598dc : Drop the dependency on python-trololio

More detail in the patch description

For the point 2, you can see: https://pagure.io/pagure/pull-request/5043 as well

Thanks for the patch. @wombelix could you also take a look at this?

For the point 2, you can see: https://pagure.io/pagure/pull-request/5043 as well

I am surprised that the issue was discussed already 2 years ago as Pagure have been running on Fedora here for years.

Jenkins Build #161 failed, I looked at the output and not convinced that the breakage is caused by commit 49df7760

When running the tests locally there is a huge number of warnings, for example:

tests/__init__.py:13
  /home/kk/tmp/pagure/tests/__init__.py:13: DeprecationWarning: the imp module is deprecated in favour of importlib and slated for removal in Python 3.12; see the module's documentation for alternative uses
tests/__init__.py:import imp
tests/__init__.py:        imp.reload(pagure.lib.tasks)
tests/__init__.py:        imp.reload(pagure.lib.tasks_mirror)
tests/__init__.py:        imp.reload(pagure.lib.tasks_services)

Relevant(?) tests doesn't error:

  • tests/test_stream_server.py
  • tests/test_pagure_repospanner.py
  • tests/test_pagure_send_notification.py
  • tests/test_pagure_lib_notify.py

The error detected by tests/test_pagure_lib_gitolite_config.py looks pretty serious:

self = <celery.backends.redis.RedisBackend object at 0x7f1280acdb10>, task_id = None, key = ''
    def get_key_for_task(self, task_id, key=''):
        """Get the cache key for a task by id."""
        if not task_id:
>           raise ValueError(f'task_id must not be empty. Got {task_id} instead.')
E           ValueError: task_id must not be empty. Got None instead.

and warnings like the one below from almost all the tests, make me nervous:

tests/test_pagure_lib_git_auth_paguregitauth.py::PagureLibGitAuthPagureGitAuthtests::test_cases
  /home/kk/tmp/pagure/tests/../pagure/lib/model.py:107: SAWarning: relationship 'BoardStatus.boards_issues' will copy column board_statuses.id to column boards_issues.status_id, which conflicts with relationship(s): 'Board.boards_issues' (copies board_statuses.id to boards_issues.status_id). If this is not the intention, consider if these relationships should be linked with back_populates, or if viewonly=True should be applied to one or more if they are read-only. For the less common case that foreign key constraints are partially overlapping, the orm.foreign() annotation can be used to isolate the columns that should be written towards.   To silence this warning, add the parameter 'overlaps="boards_issues"' to the 'BoardStatus.boards_issues' relationship. (Background on this error at: https://sqlalche.me/e/14/qzyx)
    ticket_stat = StatusIssue(status=status)

I see both these messages in the log files on the server, ... gigabytes(!) of references to https://sqlalche.me/e/14/qzyx!).

Thanks for working on this @klauskoder, I would feel more comfortable when we first update the unit tests to f38 (see: https://pagure.io/pagure/pull-request/5396).

One thing about:

    The requirements.txt file specify max-versions of practically
    all packages, removing them cause no trouble on fc38.

We all agree it's not great to have to max-version boundaries, but they needed to be added for good reasons to get the tests working again (see https://pagure.io/pagure/pull-request/5365). It would be great to get rid of them but we still have a lot tech depts and I think there is more to do before that's possible.

Let' me dive a little deeper in your patch and do some tests.

https://jenkins-pagure.apps.ocp.cloud.ci.centos.org/job/pull-requests/161/consoleText was just running the F36 rpm tests:

[2023-09-13T05:58:27.619Z] STEP 1/12: FROM quay.io/fedora/fedora:36-x86_64

Indeed only three failed:

[2023-09-13T06:16:31.462Z] =========================== short test summary info ============================
[2023-09-13T06:16:31.462Z] FAILED tests/test_pagure_flask_api.py::PagureFlaskApitests::test_api_error_codes
[2023-09-13T06:16:31.462Z] FAILED tests/test_style.py::TestStyle::test_code_with_black - AssertionError:...
[2023-09-13T06:16:31.462Z] FAILED tests/test_style.py::TestStyle::test_code_with_flake8 - AssertionError...

But f36 pip and centos8 didn't run, as soon one failed the others will not be tried as far I remember.
So we can assume they are way more errors for those systems coming up.

Every F is a failed test, ongoing run local on CentOS 8 Stream, I'm quite sure that most of them are caused because the python package pinning was removed:

============================================================================================================================ test session starts =============================================================================================================================
platform linux -- Python 3.6.8, pytest-3.4.2, py-1.5.3, pluggy-0.6.0
rootdir: /pagure, inifile:
plugins: xdist-1.24.1, forked-1.0.2, celery-4.3.0
gw0 [1711] / gw1 [1711] / gw2 [1711] / gw3 [1711] / gw4 [1711] / gw5 [1711] / gw6 [1711] / gw7 [1711]
scheduling tests via LoadScheduling
............................................................................................F............FF....................................................................................................F...F.FF.FF..F.FF.F.FF.FF..F..F..F.F................... [ 15%]
......................................................................................................................................................................F........F..F......F..F..F......FF.....F....F.F...FF..F....F.FF...FF....F...F..FF.....F.F.....FF [ 30%]
.F....FFF....FF.F...F.F.F...FFF....F.F...FF.FF....FF.F...F.F.F....FF.F.FF.F..F.FF....F.F.F...F.F....F.F..F..F.FF...FF.F....F..F...F.FF....FF..FF.F.F..F.FFF...FFF..FF.F.FF..F..FF.FF..FF..FF.F.F..FF....FF.F..F.F..F..F..F..FF....F...F....F...FFF..FF.F...F.F...FFF.F [ 45%]
..FFF.F.FF...FFFFF.FFF.FFFF.F.FFFFFF.FF..FFFFF...FFF..FFF.F.FFFF.FFFFFFFFFF.FFF.FF.FFFFFFFF.FF.FFFF.FFF.FFFFFF.FFF..FFFFFFFFFF.FFFFFFFFFF.FFFFFFFFFF.FFFFFFFFFFFFF.FFFFFFFFFFFFFFFF.F.FFFF.F.FFFFFFF.FFFF.FFFF.FFFF.F.FF.FFFFF.FFF.FFF.FFF.FFFFF.FFFF.F.FF..F.FFFF..FF [ 61%]
FFFFFFFFFFFF.FFFFFFFF.FFFF.F.FFFF.FFFF.FFFF.FF.FFF.FF.FFFFFFFFFFFFFF.FFF.FF.FFFFFFF.FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF.F.F.FFFFFFFFFFFFF.FFFFFFFFsFFFFFFFFFFFFF.FFF.FFF.FFFFFF.

I will come back with some suggestions how to move forward as soon I tested the patch on all variants and put the package pinning back in place, we shouldn't change too many things at once, removing the async stuff as part of this PR is fine but let's don't open a can of worms.

We are, regarding the task_id is None problem, good with celery <= 5.2.6, it fails for example with python3-celery-5.3.1-1.fc38.noarch, I put it here for me as reminder to further investigate changes between those releases and to troubleshoot this problem in a little more detail.

Regarding the PR in general, it's with the overall goal very similar to https://pagure.io/pagure/pull-request/5396 and I think we some atomic steps to achieve it. To not introduce even more problems and things we have to work on in parallel, we should keep the package pinning for now. Obviously it need to be aligned with the rpm versions that F38 ships, which will already produce some fallout that need to be fixed, that's the approach I see in 5396. First it's all about getting the unit tests moved to F38, afterwards we can work on any bugs that the tests might not brought up or further cleanup/improvement.

Let me try to find some time within the next days to do some troubleshooting, the state of https://pagure.io/pagure/pull-request/5396 looks promising already, so focusing on the one celery related problem seems the best next step.

rebased onto dba87d385a4f57acd3218aea0df10d11f0aed478

I rebased on top of @zlopez work. Had to fix one or two package versions where the pinned version had already gone away. Added a few missing packages to requirements-fedora.txt.

Pagure is still functional, but quite noisy - that is even after my attempt in commit 05af957, to silence model.py relationship conflicts. It works, but force a hard fork on sqlme.

Did some testing with mock - a bit discouraging:

1) centos8 crash (jinja2)
2) centos9 doesn't seem to have python3-arrow, python3-binaryornot, python3-bleach, python3-flask-oidc, python3-straight-plugin, python3-celery
3) rocky+epel-9, same as centos9
4) Fedora-39, use python 3.12.0, which means that all the python packages that depend
on imp now bail out: ModuleNotFoundError: No module named 'imp'.

The problems in Fedora-39 seem to be resolved when the imp related issues discussed in commit 21524763 are fixed.

   sed -i 's/^from imp impo.*$//'  /usr/lib/python3.12/site-packages/straight/plugin/loaders.py

1 new commit added

  • Correct celery -A option, fedora requirements

I solved my original problem and Pagure is now working without exceptions or
endless burps of not-so-helpful hints on my teams fedora 38 server.
i.e It is now possible to interact with pagure from the command-line again.

This is the hacks that was required:

Patch celery

/usr/lib/python3.11/site-packages/celery/backends/base.py

 def get_key_for_task(self, task_id, key=''):
         """Get the cache key for a task by id."""
         if not task_id:
+            return None
-            raise ValueError(f'task_id must not be empty. Got {task_id} instead.')
         return self._get_key_for(self.task_keyprefix, task_id, key)

Patch straight

The python module straight.plugin.loaders import imp.find_module,
from obsoleted imp. It is not used so remove:

/usr/lib/python3.11/site-packages/straight/plugin/loaders.py:

- from imp import find_module
+ #from imp import find_module

Install squalchemy1.3 instead of squalchemy1.4

Pagures model.py is not compatible with sqlalchemy-1.4,
luckily Fedora 38 ship an older version:

dnf install --allowerasing python3-sqlalchemy1.3

PEP 494 – Python 3.6 Release Schedule | peps.python.org
May 30, 2015 · As of 2021-12-23, 3.6 has reached the
end-of-life phase of its release cycle. 3.6.15 was the
final security release.

Maybe it is time to move on?

pretty please pagure-ci rebuild

Appreciate the time and effort you invested @klauskoder, let's see what the results of a new test run are.

For obvious reasons, we can't patch /usr/lib/python3.11/site-packages/celery/backends/base.py and /usr/lib/python3.11/site-packages/straight/plugin/loaders.py as you did and need to find a different solution that can be shipped with pagure.

@klauskoder can you please rebase on latest master?

[2023-09-23T10:52:56.142Z] HEAD is now at 9762a035 Update chatroom reference to the new official Matrix room
[2023-09-23T10:52:56.142Z] + git config --global user.email you@example.com
[2023-09-23T10:52:56.142Z] + git config --global user.name 'Your Name'
[2023-09-23T10:52:56.142Z] + git merge --no-ff proposed/build-fedora38 -m 'Merge PR'
[2023-09-23T10:52:56.142Z] fatal: refusing to merge unrelated histories

You PR is on https://pagure.io/pagure/c/d11ec7e9efb10aa2f075debc0574579e2fb46b54?branch=master and not https://pagure.io/pagure/c/9762a035ed9bbe19317294e76fbae9f2e6b1e6bf?branch=master, causes the pipeline to fail, thanks.

I did some local tests based on the latest code from this PR, results (F38 rpm): 314 failed, 1390 passed, 8 skipped, 2591 warnings in 2842.89s (0:47:22)

The issue that straight.plugin imports from imp without actually using it, has an open PR (https://github.com/ironfroggy/straight.plugin/pull/30) since June 2022, so I wouldn't expect that we see that fixed soon.
Only warning that shows up in the unit tests: It will be deprecated in Py3.12, right now it's more of a cosmetic issue for us compared to all the other topics we have to address tbh:

  /usr/lib/python3.11/site-packages/straight/plugin/loaders.py:7: DeprecationWarning: the imp module is deprecated in favour of importlib and slated for removal in Python 3.12; see the module's documentation for alternative uses
    from imp import find_module

More serious is the strict error handling of an empty key that was introduced with https://github.com/celery/celery/commit/726b664840b6a1fcea9225b254a393e665363ad0 and is part of celery releases since 5.3.0.

     def get_key_for_task(self, task_id, key=''):
         """Get the cache key for a task by id."""
-        key_t = self.key_t
-        return key_t('').join([
-            self.task_keyprefix, key_t(task_id), key_t(key),
-        ])
+        if not task_id:
+            raise ValueError(f'task_id must not be empty. Got {task_id} instead.')
+        return self._get_key_for(self.task_keyprefix, task_id, key)

This causes a large amount of errors like this one in the unit tests:

___________________________________________________________________________________________________________________ TestCommitsAuthorStats.test_no_change ____________________________________________________________________________________________________________________
[gw3] linux -- Python 3.11.5 /usr/bin/python3
self = <tests.test_tasks.TestCommitsAuthorStats testMethod=test_no_change>
    def test_no_change(self):
        self.commits = [
            MockCommit("Alice", "alice@example.com", "2018-01-01 00:00")
        ]
        self.authors = {
            "alice@example.com": MockUser("Alice", "alice@example.com")
        }
        (
            num_commits,
            authors,
            num_authors,
            last_time,
>       ) = tasks.commits_author_stats(self.repopath)
tests/test_tasks.py:67: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/lib/python3.11/site-packages/celery/local.py:182: in __call__
    return self._get_current_object()(*a, **kw)
/usr/lib/python3.11/site-packages/celery/app/task.py:411: in __call__
    return self.run(*args, **kwargs)
pagure/lib/tasks_utils.py:31: in decorated_function
    self.update_state(state="RUNNING")
/usr/lib/python3.11/site-packages/celery/app/task.py:999: in update_state
    self.backend.store_result(
/usr/lib/python3.11/site-packages/celery/backends/base.py:526: in store_result
    self._store_result(task_id, result, state, traceback,
/usr/lib/python3.11/site-packages/celery/backends/base.py:973: in _store_result
    current_meta = self._get_task_meta_for(task_id)
/usr/lib/python3.11/site-packages/celery/backends/base.py:995: in _get_task_meta_for
    meta = self.get(self.get_key_for_task(task_id))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <celery.backends.redis.RedisBackend object at 0x7f68e704eb10>, task_id = None, key = ''
    def get_key_for_task(self, task_id, key=''):
        """Get the cache key for a task by id."""
        if not task_id:
>           raise ValueError(f'task_id must not be empty. Got {task_id} instead.')
E           ValueError: task_id must not be empty. Got None instead.
/usr/lib/python3.11/site-packages/celery/backends/base.py:869: ValueError

Tracked down to our decorator function @pagure_task in pagure/lib/task_utils.py, this can be solved by passing the the self object to update_state in line 31:

- self.update_state(task_id=self.request.id, state="RUNNING")
+ self.update_state(state="RUNNING")

At this point, there is no task running and therefore no id assigned to self.request.id yet.
By passing self, the request id will be added right after the task was started as part of the update_state call.
All task_id related errors are gone in the unit tests after this change.

Results of another test run (again only F38 rpm): 268 failed, 1436 passed, 8 skipped, 2603 warnings in 2937.65s (0:48:57)

There are still a lot of failed tests that I have to check later.
Some of them are related to sqlalchemy, so I assume they are gone in F38 rpm after installing 1.3 instead 1.4 as outlined by @klauskoder:

dnf install --allowerasing python3-sqlalchemy1.3

But I see also quite a lot related to a version mismatch of pygit like this one:

E               pkg_resources.VersionConflict: (pygit2 1.12.2 (/usr/lib64/python3.11/site-packages), Requirement.parse('pygit2<=1.12.0,>=0.26.0'))

to be continued.

I tried to reproduce the fix to task_id == None issue / update_state() as you describe. But I can't make it work.

The two calls to update_state() you show

self.update_state(state="RUNNING")
self.update_state(task_id=self.request.id, state="RUNNING")

seems equivalent for all values of self.request.id:

as:

    def update_state(self, task_id=None, state=None, meta=None, **kwargs):
        """Update task state.
        Arguments:
            task_id (str): Id of the task to update.
                Defaults to the id of the current task.
            state (str): New state.
            meta (Dict): State meta-data.
        """ 
        if task_id is None:
            task_id = self.request.id
        self.backend.store_result(

rebased onto 9cfc8d8d0c94be3eceb1f0505de1e6520370d746

The two calls to update_state() you show

self.update_state(state="RUNNING") self.update_state(task_id=self.request.id, state="RUNNING")

Sorry, total nonsense I commented there... That was the first attempt when I realized that self.request.id is empty at this stage.

What I meant and worked for me / satisfied the unittest was to replace:

self.update_state(state="RUNNING")

with

self.update_state(self=self, state="RUNNING")

So I pass the self obj to the update_state method.

What I struggle with is to reproduce your sqlalchemy workaround by installing 1.3 instead of 1.4, when I do that in the f38 container and run the unit tests, ALL of them (~500) fail. I have no logical explanation right now why that seem to work on your pagure server but kills all unit test.
Edited by wombelix 
jenkins Build #168 failed (commit: 6b3eaeee) Sun, 24 Sep 2023 16:54:16 GMT
314 failed, 1391 passed, 7 skipped, 2626 warnings in 1155.47s (0:19:15)

Looks very similar to the tests I was running locally based on a f38 container, by applying the task_id fix to this PR, it can most likely dropped to ~268 failed, but still quite some work ahead to get the unit tests back to green when moving to f38.

If I read the log of Build #168 correctly, then it seems that almost everything fail with

"ValueError: task_id must not be empty. Got None instead."

If I read the log of Build #168 correctly, then it seems that almost everything fail with

"ValueError: task_id must not be empty. Got None instead."

Yep, you didn't pushed a new commit with self.update_state(self=self, state="RUNNING") to this PR, so here in the CI we will still see all those errors. Tests in my local environment are fine on f38, at least for the task_id issue, all the other things I mentioned are still there and need further investigation.

7 new commits added

  • Fix "empty git" test cases
  • Fix AssertionError in test_api_error_codes
  • test_style: AssertionError
  • Fix create_remote() DeprecationWarning
  • Fix warning jinga2.escape DeprecationWarning
  • Fix self.assert(Not)Equals() DeprecationWarning
  • Fix Celery related exception in get_key_for_task

After applying @wombelix s fix, the score stands at: 59 warnings and 7 errors

52 warnings about Crochet use of isSet():

  /usr/lib/python3.11/site-packages/crochet/_eventloop.py:{160,154}: DeprecationWarning: isSet() is deprecated, use is_set() instead

1 warning in Crochet about use deprecated imp module

  /usr/lib/python3.11/site-packages/crochet/_eventloop.py:13: DeprecationWarning: the imp module is deprecated

and 3 warnings from sqlalchemy, see below

This add up to a total of 56 warnings ... the delta of 3 seem to be double counting of sqlalchemy warnings.

-----------------------------------------
tests/test_pagure_flask_ui_repo.py::PagureFlaskRepotests::test_delete_repo
  /usr/lib64/python3.11/site-packages/sqlalchemy/orm/session.py:1952: SAWarning: Identity map already had an identity for (<class 'pagure.lib.model.Project'>, (4,), None), replacing it with newly flushed object.   Are there load operations occurring inside of an event handler within the flush?
tests/test_pagure_flask_ui_repo.py::PagureFlaskRepotests::test_delete_repo_with_group
tests/test_pagure_lib_model_delete_project.py::DeleteProjectTests::test_delete_project_with_group
  /usr/lib64/python3.11/site-packages/sqlalchemy/orm/persistence.py:1359: SAWarning: DELETE statement on table 'projects_groups' expected to delete 1 row(s); 0 were matched.  Please set confirm_deleted_rows=False within the mapper configuration to prevent this warning.
tests/test_pagure_flask_ui_repo.py::PagureFlaskRepotests::test_delete_repo_with_users
tests/test_pagure_lib_model_delete_project.py::DeleteProjectTests::test_delete_project_with_user
  /usr/lib64/python3.11/site-packages/sqlalchemy/orm/persistence.py:1359: SAWarning: DELETE statement on table 'user_projects' expected to delete 1 row(s); 0 were matched.  Please set confirm_deleted_rows=False within the mapper configuration to prevent this warning.
-----------------------------------------

The failed cases at this point are:

FAILED tests/test_pagure_flask_api.py::PagureFlaskApitests::test_api_error_codes - AssertionError: 47 != 46
FAILED tests/test_pagure_flask_internal.py::PagureFlaskInternaltests::test_get_stats_commits_empty_git - AssertionError: False is not true
FAILED tests/test_pagure_flask_internal.py::PagureFlaskInternaltests::test_get_stats_commits_trend_empty_git - AssertionError: False is not true
FAILED tests/test_pagure_flask_ui_fork.py::PagureFlaskForktests::test_request_pull_close - AssertionError: unexpectedly None
FAILED tests/test_pagure_flask_ui_repo.py::PagureFlaskRepotests::test_renew_api_token - AssertionError: datetime.date(2024, 3, 29) != datetime.date(2024, 3, 30)
FAILED tests/test_pagure_hooks_pagure_hook.py::PagureHooksPagureHooktests::test_generate_revision_change_log_full_url - AssertionError: expected call not found.
FAILED tests/test_pagure_hooks_pagure_hook.py::PagureHooksPagureHooktests::test_generate_revision_change_log_full_url_fork - AssertionError: expected call not found.
FAILED tests/test_pagure_hooks_pagure_hook.py::PagureHooksPagureHooktests::test_generate_revision_change_log_short_url - AssertionError: expected call not found.
===== 8 failed, 1697 passed, 7 skipped, 59 warnings in 13634.17s (3:47:14) ======

I added the missing APIERROR enum to the PagureFlaskApitests test case and re-ran all
the failing tests (A full test run takes 3 hours to run). Only 2 failures are flagged:

FAILED tests/test_pagure_flask_internal.py::PagureFlaskInternaltests::test_get_stats_commits_empty_git - AssertionError: False is not true
FAILED tests/test_pagure_flask_internal.py::PagureFlaskInternaltests::test_get_stats_commits_trend_empty_git - AssertionError: False is not true

These failures are mysterious, test_get_stats_commits_trend_empty_git and test_get_stats_commits_empty_git both follow this pattern:

def test_get_stats_commits_empty_git(self):
[]
    output = self.app.post("/pv/stats/commits/xxxx", data=data)

Those calls end up in commits_history_stats() and commits_author_stats() where they call
head.peel() on a pygit2.Repository() object.

    for commit in repo_obj.walk(
        repo_obj.head.peel().oid.hex, pygit2.GIT_SORT_NONE
    ):
         # For each commit record ...

Here the peel call in turn throw an exception (as expected?) How did this code ever work without a try:?

All test pass after moving the repo_obj.head.peel().oid.hex inside a try:.

$ pytest  tests/test_pagure_flask_api.py  \
                tests/test_pagure_flask_ui_repo.py \
                tests/test_pagure_flask_internal.py  \
                tests/test_pagure_flask_ui_fork.py \
                tests/test_pagure_hooks_pagure_hook.py  \
                tests/test_pagure_flask_ui_login.py  tests/test_style.py
============================= test session starts ==============================
platform linux -- Python 3.11.2, pytest-7.2.2, pluggy-1.0.0
rootdir: /tmp/pagure
plugins: anyio-3.5.0
collected 232 items
tests/test_pagure_flask_api.py ...........
tests/test_pagure_flask_ui_repo.py .....................................
tests/test_pagure_flask_internal.py ..........................................
tests/test_pagure_flask_ui_fork.py ...............................................................
tests/test_pagure_hooks_pagure_hook.py ...
tests/test_pagure_flask_ui_login.py ...............
tests/test_style.py ...                                                  [100%]
=============================== warnings summary ===============================

There seem to be a Heisenbug in the test environment. A full test just completed and surprisingly some of the test-cases that pass when run independently failed

============================ short test summary info ============================
FAILED tests/test_pagure_flask_ui_fork.py::PagureFlaskForktests::test_request_pull_close - AssertionError: unexpectedly None
FAILED tests/test_pagure_hooks_pagure_hook.py::PagureHooksPagureHooktests::test_generate_revision_change_log_full_url - AssertionError: expected call not found.
FAILED tests/test_pagure_hooks_pagure_hook.py::PagureHooksPagureHooktests::test_generate_revision_change_log_full_url_fork - AssertionError: expected call not found.
FAILED tests/test_pagure_hooks_pagure_hook.py::PagureHooksPagureHooktests::test_generate_revision_change_log_short_url - AssertionError: expected call not found.
=== 4 failed, 1701 passed, 7 skipped, 59 warnings in 13656.01s (3:47:36) ===

1 new commit added

  • Fix PagureHooksPagureHooktests failure when system wide config exist

Jenkins build 170 fail with VersionConflict:

17:40:14      raise VersionConflict(dist, req).with_context(dependent_req)
17:40:14  pkg_resources.VersionConflict: (SQLAlchemy 1.4.49 (/usr/lib64/python3.11/site-packages), Requirement.parse('sqlalchemy<=1.4.46,>=0.8'))

The sqlalchemy version 1.4.46 is (IMHO) not anything scientific, but rather the version that fc38 had at the time @zlopez made the original pull-request. (like the version of pygit2 in comment https://pagure.io/pagure/pull-request/5405#comment-192673)

sqlalchemy version 1.3 is required to solve the chatty warnings of versoin 1.4.
I am not sure how to specify that sqlalchemy version 1.3 should be used. I added it to the
requirements-fedora.txt file so the rpms build correctly:

diff --git a/requirements-fedora.txt b/requirements-fedora.txt
-python3-sqlalchemy
+python3-sqlalchemy1.3

I updated the requirements.txt file accordingly in commit b3eaeeeb so pip should work

b3eaeeeb (Klaus Koder        2023-09-13 12:21:02 +0800 36) sqlalchemy >= 0.8, <= 1.3.24

fantastic , I got my pagure server back and working on Fedora 38 , the universaldetector version now is 5.2.0 [1] so after update encoding_utils.py [2] , it is working very well , with your patches thank you for doing this .

[1]
In [4]: ch_version
Out[4]: '5.2.0'

--- /home/sergio/fedora-scm/pagure/pagure.git/pagure/lib/encoding_utils.py      2023-01-16 11:29:18.430838387 +0000
+++ /usr/lib/python3.11/site-packages/pagure/lib/encoding_utils.py      2023-10-14 14:35:44.492627244 +0100
@@ -68,7 +68,7 @@ def detect_encodings(data):
if cchardet:
return encodings
-    if ch_version[0] in ("3", "4"):
+    if ch_version[0] in ("3", "4", "5"):
for prober in detector._charset_probers:
if hasattr(prober, "probers"):
for prober in prober.probers:

@klauskoder thanks for all your hard work on this PR. great progess! When I take a look into the latest CI run, I see:

=== 265 failed, 1440 passed, 7 skipped, 1078 warnings in 1139.92s (0:18:59) ====

The CI is configured in a way that in case of a failed test, the following operating system will not be tested anymore. Means that the Cent OS 8 stream test throws 265 errors and we not see what happens with F38 rpm and pip.

I know that you focused on F38, but we need to take all three unit test variations (c8s rpm, f38 rpm, f38 pip) into account and ensure they are without failures before we can merge the code.

I will run local tests to see if I can reproduce your f38 pip results with just a couple of errors left, that would be a great starting point to hopefully get the rest under control soon.

With almost 4h runtime for a full test, I understand that it's not really pleasant experience for you, it "only" takes ~40min on my machine and that's already long when you hunt such a bug.

@sergiomb thanks, I will incorporate your patch in my tests as well.

@klauskoder can you help me to understand how you run the local tests in your environment?

I pulled your latest changes, execute ./run-tests-container.py --rebuild inside the dev folder and receive the following results:

CentOS 8 Stream RPM: 1016 error in 123.43 seconds

F38 RPM: 265 failed, 1439 passed, 8 skipped, 997 warnings in 2295.44s (0:38:15)

F38 PIP: 127 errors in 10.20s

py38: exit 1 (10.57 seconds) /pagure> pytest -n auto tests/ pid=62
py38: FAIL code 1 (37.72=setup[27.16]+cmd[10.57] seconds)
evaluation failed :( (37.79 seconds)

The CI uses run_ci_tests_containers.sh, I also double checked the output of the last CI run, we test in the following order:
- F38 RPM
- F38 PIP
- CentOS 8 Stream RPM

Which means, because of failed tests in F38 RPM, F38 PIP and CentOS 8 Stream doesn't run in Jenkins.
Also, the local tests (265 failed, 1439 passed, 8 skipped, 997 warnings) match nearly the last CI run (265 failed, 1440 passed, 7 skipped, 1078 warnings) but are far away from what you seem to see local (4 failed, 1701 passed, 7 skipped, 59 warnings).

Another challenge we have: By default, c8s comes with Python 3.6.8, f38 with Python 3.11.4 and as of today we use Python 3.8 for our pip based tests. As far I can see, Python 3.9 is available to install on c8s and f38 but also openSUSE Leap 15.5 and (RH)EL8 and is default on (RH)EL9. Maybe we should consider this as our common ground and not try to still support Python 3.6 @ngompa ?
This will not solve immediately all our issues but probably make things easier when we only need to find something that works on 3.9 and 3.11 instead of 3.6 all the way up to 3.11.
Edited by wombelix

I don't have a problem with lifting up to Python 3.9 for Pagure 6.0. I think openSUSE Leap builds complete stacks, but even if it doesn't, I can build the missing dependencies for openSUSE infrastructure easily enough.

I don't intend to ship Pagure 6.0 to RHEL 8 and older anyway.

Metadata Update from @wombelix:
- Request assigned

Metadata Update from @wombelix:
- Request assigned

@klauskoder thanks again for your contribution. I picked a couple of your commits from this PR and added them to https://pagure.io/pagure/pull-request/5463. I will close this PR in favour of the new one.

Pull-Request has been closed by wombelix
Metadata
wombelix
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.