katello
katello copied to clipboard
Katello
Feature/debian errata working
What are the changes introduced in this pull request?
It is basically PR #7961 but with some changes to make it work with foreman 4.3.0 rc4.
It introduces a table column nva for tables katello_debs and katello_erratum_deb_packages to calculate the errata for a package more quickly and easily. The rest are minor adjustments.
Considerations taken when implementing this change?
The changes are made for a specific customer of our company. So the changes might not be completely suitable for the merge. But we thought we share our changes to help the progress with the debian errata feature.
What are the testing steps for this pull request?
We did manual testing on several real systems. The contract does not have budget for automated testing in katello, sorry.
Looks like your branch includes way too many commits. Could you rebase this on master?
Looks like your branch includes way too many commits. Could you rebase this on master?
Sorry, it was based on 4.3.0 RC 4. I force-pushed a rebase.
@melcorr I think I closed it since there has not been any updates and I was closing out some stale prs. The user can reopen it if they want.
@chris1984 What updates did you expect? Since I opened this PR, I received pretty much no feedback...
@melcorr Well, we have that code working and are using it in production.
Basically, you need to review whether this code is suitable for your standards and if you want to include it into katello.
@chris1984 can you open this PR again? Since I've been involved in the Foreman community, people have been asking for this. I would appreciate if Katello folks could review. This is potentially a very valuable contribution.
@melcorr reopened and can take a look and add it to the Jira board to review. @mbgonicus can you rebase so we can start the review process. Sorry we missed it.
I'm also really looking forward to the deb Errata feature. But sorry for asking, what was the motivation behind improving the changes from the ATIX team and creating a separate PR (with a single commit) instead of getting in touch with them and integrating the changes in the already present PR?
I have been following the other PR since a while now, and it looks like there is still work done. I'm not really saying this is a bad thing, but maybe it would help to get to the best solution with joint forces.
I'm also really looking forward to the deb Errata feature. But sorry for asking, what was the motivation behind improving the changes from the ATIX team and creating a separate PR (with a single commit) instead of getting in touch with them and integrating the changes in the already present PR?
I have been following the other PR since a while now, and it looks like there is still work done. I'm not really saying this is a bad thing, but maybe it would help to get to the best solution with joint forces.
Well we had that deb errata as a customer request. Their system was a little special due to customizations and the other PR was not finished. Because it had to be done quickly (within 2 days) we just hacked our way through. But since it is working, we just wanted to give the code to you, hoping it might help.
So if you want to modify this code or just pick some stuff to the other PR, please feel free to do so.
@chris1984 I'll try to find some time to rebase it
I'm also really looking forward to the deb Errata feature. But sorry for asking, what was the motivation behind improving the changes from the ATIX team and creating a separate PR (with a single commit) instead of getting in touch with them and integrating the changes in the already present PR? I have been following the other PR since a while now, and it looks like there is still work done. I'm not really saying this is a bad thing, but maybe it would help to get to the best solution with joint forces.
Well we had that deb errata as a customer request. Their system was a little special due to customizations and the other PR was not finished. Because it had to be done quickly (within 2 days) we just hacked our way through. But since it is working, we just wanted to give the code to you, hoping it might help.
So if you want to modify this code or just pick some stuff to the other PR, please feel free to do so.
@chris1984 I'll try to find some time to rebase it
Yes exactly, and everything I meant with that message was, why it wasn't reached out to the team who already has the PR running and asking them if they want the additional changes, and integrate them in their PR (which is possible to send an PR against the PR in the ATIX repo) Because they will most likely also be the once who will improve and further manage that part if I look in the history of the commits.
(I'm just a user not one of their team, sorry if it looked like that)
@lumarel over the last two years, I have been annoying @sbernhard for updates on the other PR. I think that the migration to Pulp 3 ate into a lot of that team's time and resources. I don't think that there was capacity for this, or at least that was what was reported. @quba42 gave a roadmap update at our birthday celebration last year and I'm pretty sure he reported the same. I hope that ATIX can build on this if we get it merged.
@melcorr It's definitely not an easy task to integrate all these functions, as far as I have seen that already included like 5 PRs in 4 projects (or even more), so I'm really grateful that they are taking so much time to upstream their effort! :)
It's just that I can see like 3 commits from a months ago, which are not part of this one (additionally to this being a single commit with the content of some of the commits of the other PR). And maybe some of the additional stuff here would make it possible to merge the other PR quite soon, because @mbgonicus already thought about the parts which were not polished in the other one!
I have trouble solving the merge conflicts. I never understood a lot of this project and cannot judge what would be correct.
However, I reckon we need someone with in-deep knowledge to have a look into this and the other PR who can evaluate and sum up the "correct" way of implementing this feature.
Could someone post a status update for this issue? Perhaps is a final release date at the horizon?