Re: [Launchpad-reviewers] [Merge] ~pelpsi/launchpad:fetch-service-session-API into launchpad:master

2024-03-06 Thread Simone Pelosi
Diff comments: > diff --git a/lib/lp/buildmaster/builderproxy.py > b/lib/lp/buildmaster/builderproxy.py > index be65e29..a1d475b 100644 > --- a/lib/lp/buildmaster/builderproxy.py > +++ b/lib/lp/buildmaster/builderproxy.py > @@ -52,26 +67,28 @@ class BuilderProxyMixin: > > end

Re: [Launchpad-reviewers] [Merge] ~pelpsi/launchpad:fetch-service-session-API into launchpad:master

2024-03-06 Thread Jürgen Gmach
Thanks! Looks good now! I would ask you to revert the refactoring when reading the configuration, or at very least put the refactoring and the new features in two separate commits. Also, I left again the question where "FetchServiceURLMatcher" is used? Diff comments: > diff --git a/lib/lp/buil

Re: [Launchpad-reviewers] [Merge] ~pelpsi/launchpad:fetch-service-session-API into launchpad:master

2024-03-05 Thread Jürgen Gmach
Thanks a lot for the MP! We are on the right track! As we went with the approach to handle both cases inside a single mixin, we really, really want to have a good class docstring to explain that there are two proxies involved. Also we should add a comment or a paragraph in the docstring which d

Re: [Launchpad-reviewers] [Merge] ~pelpsi/launchpad:fetch-service-session-API into launchpad:master

2024-03-05 Thread Simone Pelosi
Diff comments: > diff --git a/lib/lp/buildmaster/downloader.py > b/lib/lp/buildmaster/downloader.py > index fc16852..cee308a 100644 > --- a/lib/lp/buildmaster/downloader.py > +++ b/lib/lp/buildmaster/downloader.py > @@ -94,3 +110,24 @@ class RequestProcess(AMPChild): > ) >

Re: [Launchpad-reviewers] [Merge] ~pelpsi/launchpad:fetch-service-session-API into launchpad:master

2024-03-04 Thread Ines Almeida
Diff comments: > diff --git a/lib/lp/buildmaster/downloader.py > b/lib/lp/buildmaster/downloader.py > index fc16852..cee308a 100644 > --- a/lib/lp/buildmaster/downloader.py > +++ b/lib/lp/buildmaster/downloader.py > @@ -94,3 +110,24 @@ class RequestProcess(AMPChild): > ) >

Re: [Launchpad-reviewers] [Merge] ~pelpsi/launchpad:fetch-service-session-API into launchpad:master

2024-03-04 Thread Simone Pelosi
Thank you for the review Ines, I'm so sorry I forgot to push the commit in which I removed all the debugging stuff or useless comments :P -- https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/461721 Your team Launchpad code reviewers is requested to review the proposed merge of ~

Re: [Launchpad-reviewers] [Merge] ~pelpsi/launchpad:fetch-service-session-API into launchpad:master

2024-03-04 Thread Simone Pelosi
Diff comments: > diff --git a/lib/lp/buildmaster/downloader.py > b/lib/lp/buildmaster/downloader.py > index fc16852..cee308a 100644 > --- a/lib/lp/buildmaster/downloader.py > +++ b/lib/lp/buildmaster/downloader.py > @@ -94,3 +110,24 @@ class RequestProcess(AMPChild): > ) >

Re: [Launchpad-reviewers] [Merge] ~pelpsi/launchpad:fetch-service-session-API into launchpad:master

2024-03-04 Thread Ines Almeida
Good work - left a few comments! I'll leave the approval to Jürgen who is more into the details of the whole project. Diff comments: > diff --git a/lib/lp/buildmaster/builderproxy.py > b/lib/lp/buildmaster/builderproxy.py > index be65e29..4f249a7 100644 > --- a/lib/lp/buildmaster/builderproxy.p

[Launchpad-reviewers] [Merge] ~pelpsi/launchpad:fetch-service-session-API into launchpad:master

2024-03-04 Thread Simone Pelosi
Simone Pelosi has proposed merging ~pelpsi/launchpad:fetch-service-session-API into launchpad:master with ~ines-almeida/launchpad:fetch-service-option-model-update as a prerequisite. Commit message: Add fetch service session API Update BuilderProxyMixin class to switch between Fetch Service