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

get fedbadges in a good shape again #90

Closed
3 of 6 tasks
erolkskn opened this issue Sep 28, 2022 · 11 comments
Closed
3 of 6 tasks

get fedbadges in a good shape again #90

erolkskn opened this issue Sep 28, 2022 · 11 comments
Assignees
Labels

Comments

@erolkskn
Copy link
Contributor

erolkskn commented Sep 28, 2022

This is a brief list of issues to be solved for making fedbadges operational again.

Immediate Action Required

Issues

Other

  • Since the current development state shows that source is already ported to Python3, that means we can use Datanommer's latest models in our code. I guess that there will be no need for this piece of code after that update: https://pagure.io/fedora-infra/ansible/blob/main/f/roles/badges/backend/files/datanommer.models__init__.py
  • The time it takes for awarding a badge should be logged. This way we can easily troubleshoot performance problems with the current architecture.
  • At the start,fedmsg-hub tries to fetch messages after the last time it ran. This causes systemd to restart service because of the holdoff time gets over. And infinite loops happens after that.

Needed Soon

This list is subject to change.

@erolkskn erolkskn self-assigned this Sep 28, 2022
@erolkskn erolkskn added the bug label Sep 28, 2022
@erolkskn erolkskn changed the title make fedbadges operational get fedbadges in a good shape again Sep 28, 2022
@penguinpee
Copy link

Nice start and good to hear that fedbadges is already on Python3. I went over the list and have the following to add/amend:

Just two examples of overlapping information / tickets. I would suggest to make it clear somehow, that issues should be reported in Pagure. There we can keep track of all issues in one place and delegate as needed: fedorabadges, tahrir, tahrir-api and badge configuration. I plan on using labels for that.

While it's nice to have issues in GitHub for easy referencing, I believe the common consensus is to use Pagure for Fedora Apps. It would be nice if these two could be integrated somehow. E.g. label fedorabadges is assigned in Pagure will open ticket for fedorabadges on GitHub. But that is definitely a nice to have for the longer term.

Once I get access to fedora-badges on Pagure, I will start working on open tickets and PRs.

All of the above is, of course, still up for discussion with current maintainers and admins, should they show up. Just my $0.02 for now.

@erolkskn
Copy link
Contributor Author

@penguinpee Thank you for your comment Sandro. It seems like i made an early call when i told that "Fedbadges is ported on Python3". Actually it's not in a working state right now, and it needs a little bit polishment.

@mscherer
Copy link
Contributor

IIRC, the devel branch is running on py3.

@mscherer
Copy link
Contributor

Oh wait, I misread. The CI is running python 3.11 (or 3.8), so can you clarify what is not in a working state ? Are you speaking of the whole stack, or is the coverage gap on the tests ?

@erolkskn
Copy link
Contributor Author

erolkskn commented Sep 29, 2022

Oh wait, I misread. The CI is running python 3.11 (or 3.8), so can you clarify what is not in a working state ? Are you speaking of the whole stack, or is the coverage gap on the tests ?
Hi mscherer,

I'm speaking of the whole stack. At a quick glance i could find:

with self.lock():

lock() is not a callable here. Instead of this we should use https://docs.python.org/3/library/threading.html#threading.Lock.acquire

self._initialize_tahrir_connection()

_initialize_tahrir_connection() is undefined.

from gssapi import Credentials, exceptions

Credentials is not exported module of "gssapi". Effectively it's broken.

m = re.search('^https?://([a-z][a-z0-9]+)\.id\.fedoraproject\.org$', openid)

Even though it works for now it doesn't mean always will. '\.' in strings are deprecated and considered as unsupported in python3. We should prefix the string by 'r'. see: https://bugs.python.org/issue27364

self.l.badgr_client = BadgrClient(

As an individual contributor, how do I run that project on pair with fedbadges ? There is not even a single line of documentation about that. It also probably should be noted that this project is not even in staging environment yet. No body knows how to run it except few people.

@mscherer
Copy link
Contributor

mscherer commented Sep 29, 2022

As an individual contributor, how do I run that project on pair with fedbadges ? There is not even a single line of documentation > about that

Yup, there is only the playbooks from Fedora. That's custom component for Fedora, so not surprised.

It also probably should be noted that this project is not even in staging environment yet

There is a staging instance for Fedora, so I am not sure to follow you. Or do you mean there a need for a different staging instance (eg, one with python 3) ?

So I am kinda concerned that the CI didn't report those issues. Especially the one that should have failed (like the import one).

@erolkskn
Copy link
Contributor Author

erolkskn commented Sep 29, 2022

Yup, there is only the playbooks from Fedora. That's custom component for Fedora, so not surprised.

Are you really suggesting that people should analyze the fedora-infra/ansible project in order to create a basic development environment for fedbadges ? You know there is a project called "podman-compose" and the another one "docker-compose" too. They are being used by thousands of projects for years to create basic dev environments.

There is a staging instance for Fedora, so I am not sure to follow you. Or do you mean there a need for a different staging instance (eg, one with python 3) ?

By this project i mean "badgr-server".There is not even a single line of definition for badgr-server in fedora-infra/ansible repository. Trying to integrate some project (fedbadges) with something (badgr-server) abandoned 2 years ago on primary branch is more than weird to me.

@erolkskn
Copy link
Contributor Author

You know there is a project called "podman-compose" and the another one "docker-compose" too. They are being used by thousands of projects for years to create basic dev environments for projects.

Even if the maintainer has no idea about the containerization, it could be achieved by writing a super simple Vagrantfile too like

  1. Anitya: https://github.com/fedora-infra/anitya/blob/master/Vagrantfile
  2. Noggin: https://github.com/fedora-infra/noggin/blob/dev/Vagrantfile
  3. Bodhi: https://github.com/fedora-infra/bodhi/blob/develop/Vagrantfile

@mscherer
Copy link
Contributor

While I understand your frustration, I am not the one who wrote the project, I am just trying to keep it running, nothing more. As you discovered, there is indeed plenty of things to make it easier to contribute too, and that's in itself is a contribution.
And if it was straightforward to get it fixed, the stack (tahrir + fedbadges + others) wouldn't be in a dire shape (and trust me, it was worst before )

I forgot the badgr part. We had a GSoC to port the code to run with badgr, as that was more maintained and tahrir wasn't. However, since our effort to get patches upstream never succeeded, I start to think that this is not sustainable and a better path should be to keep the existing stack (because at least, we have no need to rebase). But I do not feel like I should decide that alone.

But yes, the badgr part wasn't finished.

@erolkskn
Copy link
Contributor Author

erolkskn commented Sep 29, 2022

I might be little misunderstood here. No word i said was personal at all. If i wasn't clear about that, so sorry 😞 . I just made all these points for future reference that's it. Of course i do know that you didn't write this project and you are trying your best to keep it going, and thank you for that 😄

Anyway, as i said, if we want to keep this project going with great stability on mind, i think we should make a lot of changes to it. But these are all my opinion. I would be glad to have you here while developing it to make it better 😄

@erolkskn
Copy link
Contributor Author

Closing it since there is now an ongoing initiative to re-write fedbadges all together 😃 Most of the discussions takes place in: https://discussion.fedoraproject.org/search?q=%23badges-team

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants