fix/reposync_librepo #2942
fix/reposync_librepo #2942
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2942 +/- ##
==========================================
+ Coverage 60.49% 61.88% +1.39%
==========================================
Files 101 101
Lines 14415 14407 -8
==========================================
+ Hits 8720 8916 +196
+ Misses 5695 5491 -204
Continue to review full report at Codecov.
|
|
@tpw56j Would you mind rebasing this onto |
|
Ok, I'll do a rebase.
|
|
@tpw56j Both changes are fine by me. Feel free to take the time. I want to have this for 3.3.2 because it will be a bugfix release and this seems like a good candidate for such bugfixing! |
c70d074 to
a989e56
Compare
|
@SchoolGuy Could you please re-run the tests, I don't understand why the reposync tests failed. |
|
@tpw56j Sure. I guess it would be the best long term that we mock the actual execution of reposync to not make the test lengthy on bad internet connectivity. |
|
@tpw56j Any update on this PR? Edit: Retriggering the tests didn't help. |
|
@SchoolGuy I still can't figure out why the test fails. On my computer it works. |
|
@tpw56j If I click on the URLs you choose I indeed get a 404 from nginx on some of them. Could it be the mirror "magically" choosen by Fedora for us? |
|
@tpw56j I will take a look at the tests for you. I trust that you have tested this will in your environment and that it works. It will go into the next release! |
|
@SchoolGuy It seems to me that the python3-librepo package is missing in the development container. I'll check now. |
|
@tpw56j Wait that is indeed possible because I didn't update the dockerfile in the OBS. But why is the error message so cryptic in this case? |
|
@SchoolGuy The message that reposync returns to stderr is lost in pytest. The python3-librepo package is really missing, but it looks like something else is missing. |
|
@tpw56j Can we improve that before we add the package in OBS? Because that is really misleading for everyone? |
|
@SchoolGuy The error that reposync returns when the package is missing is no less mysterious: |
|
@tpw56j Since we are missing a python package, can't we add a helper function in the style? |
|
@tpw56j Could you please not use the |
|
@tpw56j Now we have a single failing test instead of multiple. This is great progress! Any ideas about this specific test? |
|
@SchoolGuy I haven't found a satisfactory solution to sync apt repos. Installing debmirror from the Fedora repo in OBS is not a complete solution to this problem. After making changes to OBS, |
|
@tpw56j Thanks a lot for this PR! I will review this at work tomorrow. |
cc648f4 to
3e67449
Compare
|
@tpw56j Today I will release a new version. Thus I will rebase this PR myself and clean the Git history. Thanks for your big contribution again! The failing test I will skip for now and fix at a later point. The code itself should work and that is the important thing. I believe the failures are more related to the package infrastructure then your test. |
Co-authored-by: tpw56j <smoker.tabac@gmail.com>
Co-authored-by: tpw56j <smoker.tabac@gmail.com>
Co-authored-by: tpw56j <smoker.tabac@gmail.com>
This commit is cleaning up the reposync code greatly.
- os.path.join() now is used as intended
- repo.name is not chained to a strip() anymore as spaces in names are anyway
not allowed
- Unnecessary parentheses were removed
- As commit title says: utils.die was replaces with the custom Cobbler
exception CX
Repo metadata is always deleted now because merging the metadata is prone to
errors.
Co-authored-by: tpw56j <smoker.tabac@gmail.com>
23e06a2 to
510fe90
Compare
I suggest to always delete old metadata in
.origin/repodata. When you try to partially update metadata, many problems arise. For example, #2865 and librepo trying to download the metadata that remained in the local repomd.xml, but have already been deleted in the remote repo:Fixes #2865