Remove libpagure

[lachmanfrantisek]

• Remove OurPagure.
• Implement forking logic and PR methods.
• First part of implementing the Pagure classes without libpagure.
• Add string representation for PRComment and PullRequest.

Fix #64
Fix #67

---

TODO:

• readonly mode
• tests

---

How to test?

from ogr.services.pagure import PagureService
from ogr.abstract import PRStatus


service = PagureService(token="???_PAGURE_TOKEN_???")

# Play with the API

print(service.user.get_username())
project = service.get_project(repo="colin", namespace="rpms")
print(project.get_commit_statuses("339a19b0bbc766d0c6cdbbc2ef5a32c0de9f7551")[0])
print(project.get_tags())

fork = project.get_fork()
print(fork.exists())

project.fork_create()

for pr in project.get_pr_list(status=PRStatus.all):
    print(pr)

pr = project.get_pr_info(3)
print(pr)
for c in project.get_pr_comments(3):
    print(c)

print(fork.is_fork)
print(fork.is_forked())

[lachmanfrantisek]

@jscotka
I've removed the old read-only setup for libpagure. Do you have some idea how it can be done in the current state without libpagure?
Thanks!

[rpitonak]

I played with it a little bit, and I got:

>>> project = service.get_project(repo="conu", namespace="rpms")
>>> for pr in project.get_pr_list(status=PRStatus.all):
...     print(pr)
...
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "/home/vagrant/ogr/ogr/abstract.py", line 39, in __str__
    f"PullRequest("
TypeError: 'NoneType' object is not subscriptable

[lachmanfrantisek]

@rpitonak

Good catch. Fixed.

[rpitonak]

Good catch. Fixed.

Thanks, I tried all of the commands above on conu and it worked smoothly. Nice work so far!

[lachmanfrantisek]

Thanks, I tried all of the commands above on conu and it worked smoothly.

That was just an example. Whole API should be already implemented...

[jpopelka]

Haven't tried yet

[jscotka]

few issues regarding to easier mocking of pagure API.

[jscotka]

@jscotka
I've removed the old read-only setup for libpagure. Do you have some idea how it can be done in the current state without libpagure?
Thanks!

@lachmanfrantisek by readonly mode are directly wrappend method in services/pagure, so no change here.

[lachmanfrantisek]

@lachmanfrantisek by readonly mode are directly wrappend method in services/pagure, so no change here.

Sorry, I meant the request caching...

@jscotka :

Ahh now I understand, theoretically it should be easier, to create there some class like previous our_pagure, what will contain just these methods for requests and parsing responses, and it then use inside service/pagure, and do not have request handling directly inside services/pagure, and then mocking will just replace implementation of this class. maybe it could lead also to better readability, of code.

file could be named something like: pagure_request and could contain class like Pagure what will provide API call methods,

and theoretically, as nice improvement, it could also implement generic JSON API call what will then replace https://github.com/packit-service/packit/blob/master/packit/ogr_services.py#L28 in packit ;-)

practically structure could be as it is, and then there will be in raw_request decision, that in case there is mocking, call another wrapper, so solution will be simple, just it needs to consolidate api_call and raw_request to one method, or api_call will call raw request, what will handle it then

[lachmanfrantisek]

@jscotka Can you take another look?

[jscotka]

Hi Frantisek, now it seems very well from perspective of mocking and representing request-response object. thanks a lot for these changes

[lachmanfrantisek]

Hi Frantisek, now it seems very well from perspective of mocking and representing request-response object. thanks a lot for these changes

Thanks, but the loading response values does not work now...

[lachmanfrantisek]

@jscotka Do you know, why the loading does not work and I need to dump to dict manually (c31b895)?

[lachmanfrantisek]

I have a working version with direct yaml dumping but it does not go through the pre-commit yaml validation.

@jscotka Do we want to use unsafe in pre-commit validation (only the syntax check) or use current setup?

[jscotka]

changes seems well. LGTM from perspective of mocking

[lachmanfrantisek]

OK, let's leave it as is and we can change it to object-dumping later.

[lachmanfrantisek]

@TomasTomecek @jpopelka Do you want to take a look at this? Otherwise, we can merge.

[TomasTomecek]

Let's merge and test in production :P

[jpopelka]

test in production

Do we need to do any changes in packit? I still see OurPagure in packit's tests

[lachmanfrantisek]

Do we need to do any changes in packit? I still see OurPagure in packit's tests.

Good point.

The API should state the same, but mocking probably needs some changes...

[jpopelka]

Filled new issue packit/packit#355