Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/centos anitya #2348

Merged
merged 6 commits into from
Mar 13, 2024
Merged

Conversation

mfocko
Copy link
Member

@mfocko mfocko commented Feb 14, 2024

  • Add an event class
  • Implement a parser
  • Pass around versions in a list (for New Hotness we can have just one version in the list, for Anitya version update fail)
  • Do not generate DB entry from the event
  • ⚠️ Choosing the correct version
    • Outcomes from the arch:
      • Gather more use cases
      • Require the explicit setting for version mask (for CentOS at least)
  • ⚠️ Decide whether to support Fedora for Anitya version update too… right now it's prefiltered for the CentOS

Related to #2068

@mfocko mfocko self-assigned this Feb 14, 2024
Copy link
Contributor

return None

# FIXME: Handle Fedora too in case we want to support multiple releases
# such as for Go.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was an RFE for this on the Slack already…

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, we can create a separate issue for this (e.g.this will imply monitoring settings for the package will not be considered anymore, therefore also changes in documentation etc.)

we could properly test this for CentOS Stream and once it is tested to be working do the switch for Fedora packages to not break anything for already onboarded Fedora packages

self._versions = versions

@property
def version(self) -> str:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The major issue here is that we “generate” DB entry for the release from the event object. There's also serialization involved (for Celery).

At the same time it feels a bit weird to keep logic of “deciding the next version” in the »event«.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed together with Mato that we want all the new versions, see e.g. https://apps.fedoraproject.org/datagrepper/v2/id?id=efec10ed-9f86-4e03-9e4f-2e626934b3ae&is_raw=true&size=extra-large in here we would be interested in all 3

Copy link
Member

@lbarcziova lbarcziova Feb 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and then it is up to maintainer to define the per-branch jobs and rules (e.g. via version_udpate_mask, upstream_tag_include, upstream_tag_exclude): example

and default would be the "latest greatest"

# we going to choose the version?
# a) latest greatest
# b) next unreleased?
# this can be also influenced by the mask…
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example of a problem:

  • There's a 1.0.0 release in Fedora.
  • We've got releases 1.0.1, 1.1.0 and 2.0.0 from Anitya
  • Which release do we sync? The next possible (1.0.1) or latest-greatest 2.0.0? Syncing all of them would create a lot of noise.
  • Ideal scenario here would be having the version mask defined, so it would be as explicit as possible.

Copy link
Member

@lbarcziova lbarcziova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we also have a testcase please?

@mfocko
Copy link
Member Author

mfocko commented Mar 6, 2024

recheck

Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

@mfocko
Copy link
Member Author

mfocko commented Mar 11, 2024

@lbarcziova rebased, added test case; left notes in comments (code) and also updated the original description; this should be ready for review and merge

@mfocko mfocko requested a review from lbarcziova March 11, 2024 23:19
Copy link
Member

@lbarcziova lbarcziova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the refactoring of the tests!

There are still some parts that will be done in the follow-up PRs, so
don't run at all for version update (affects no one, as it hasn't been
merged yet).

Signed-off-by: Matej Focko <[email protected]>
@mfocko mfocko added the mergeit When set, zuul wil gate and merge the PR. label Mar 13, 2024
Copy link
Contributor

Copy link
Contributor

Build succeeded (gate pipeline).
https://softwarefactory-project.io/zuul/t/packit-service/buildset/1367818e1fe249c0b9119682005fc5ab

✔️ pre-commit SUCCESS in 2m 33s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 623c117 into packit:main Mar 13, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit When set, zuul wil gate and merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants