@gnaponie Does freshmaker depend on the flask package from Fedora repositories in deployment?

Perhaps we can wait for the release of f32 and upgrade the base image to fedora:32, since python-flask-1.1.1-2.fc32 is available.

Hey @apaplaus, thank you for the PR. Could you please add in the commit message also the link to the Flask bug? So it is clear why this is a problem.

Additionally, could you remove from the commit message Freshmaker's full URL? This it is an internal URL, and this is an upstream repo. You can just write "Fixing problem when for example accessing the /builds API endpoint ....".

@gnaponie Does freshmaker depend on the flask package from Fedora repositories in deployment?
Perhaps we can wait for the release of f32 and upgrade the base image to fedora:32, since python-flask-1.1.1-2.fc32 is available.

I think flask gets installed from the pip package, there in the requirements.txt, doesn't it?
The reason why we are not waiting is because there is a bug in the current Flask version running in Freshmaker, and we want to avoid that, since it causes a strange unexpected error. Basically, when accessing an API endpoint without a trailing slash (example: /builds), you should get redirected to the right endpoint with the slash (example: /builds/), but this instead raises a 500 error because of this bug in Flask.

@apaplaus could you also add in the commit message the ref to the FACTORY jira card?
See this as example: https://pagure.io/freshmaker/pull-request/498

Also consider in the future to commit with -s option. For more info check this out: https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work

Consider using parametrized instead of having this loop: https://docs.pytest.org/en/latest/parametrize.html
Check this as an example: https://pagure.io/freshmaker/blob/master/f/tests/test_utils.py#_107

Consider using parametrized instead of having this loop: https://docs.pytest.org/en/latest/parametrize.html
Check this as an example: https://pagure.io/freshmaker/blob/master/f/tests/test_utils.py#_107

I've considered using it, but, as I understand, pytest doesn't support parametrizing of unittest.TestCase class methods(https://docs.pytest.org/en/latest/unittest.html#pytest-features-in-unittest-testcase-subclasses). And as all tests are using unittest.TestCase as parent class I've decided to just stick with simple loop.

I've considered using it, but, as I understand, pytest doesn't support parametrizing of unittest.TestCase class methods(https://docs.pytest.org/en/latest/unittest.html#pytest-features-in-unittest-testcase-subclasses). And as all tests are using unittest.TestCase as parent class I've decided to just stick with simple loop.

As discussed already "face2face", you are right, it seems like it's not possible :\
but can you maybe change:
urls = ('/api/2/builds', '/api/2/events')
to include also the version with the trailing slash:
urls = ('/api/2/builds', '/api/2/events', '/api/2/builds/', '/api/2/events/')
I know it means 2 additional steps in the loop, but at least we don't repeat code.

I've considered using it, but, as I understand, pytest doesn't support parametrizing of unittest.TestCase class methods(https://docs.pytest.org/en/latest/unittest.html#pytest-features-in-unittest-testcase-subclasses). And as all tests are using unittest.TestCase as parent class I've decided to just stick with simple loop.

As discussed already "face2face", you are right, it seems like it's not possible :\
but can you maybe change:
urls = ('/api/2/builds', '/api/2/events')
to include also the version with the trailing slash:
urls = ('/api/2/builds', '/api/2/events', '/api/2/builds/', '/api/2/events/')
I know it means 2 additional steps in the loop, but at least we don't repeat code.

Done.

Thank you :)
awesome +1

Commit 42f403ba fixes this pull-request