rpc_client: Allow http method declaration consistently with the rpc server
AbandonedPublic
Actions

Authored by ardumont on Oct 23 2020, 12:14 PM.

Details

Reviewers

None

Group Reviewers

Summary

a7d1aa7 introduced that use for the server. Without doing this consistenly in
the rpc client part, this won't work though.

The common usage we have is to define an interface in a swh module and make the
rpc client and server code out of that interface. So those needs to be
consistent.

Related to a7d1aa7

Related to P833 (as the current inconsistency, without the diff)

Test Plan

tox

Test with the vault everything is fine using docker

client code:

time doco exec swh-vault-worker /bin/bash

swh@51a13de681ec:/$ python3
Python 3.7.4 (default, Jul 13 2019, 14:04:11)
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from swh.core.config import load_from_envvar
>>> from swh.model.hashutil import hash_to_bytes
>>> from swh.vault import get_vault
>>> config = load_from_envvar()
>>> vault = get_vault(**config['vault'])
>>> vault.task_info(obj_type="directory", obj_id=hash_to_bytes("838bc7ca7541fd96ed6e665c1485a0f8b6d4c3f3"))
{'id': 1, 'type': 'directory', 'object_id': b'\x83\x8b\xc7\xcauA\xfd\x96\xednf\\\x14\x85\xa0\xf8\xb6\xd4\xc3\xf3', 'task_id': None, 'task_status': 'new', 'sticky': False, 'ts_created': datetime.datetime(2020, 10, 23, 10, 1, 21, 384052, tzinfo=datetime.timezone(datetime.timedelta(0), '+00:00')), 'ts_done': None, 'ts_last_access': datetime.datetime(2020, 10, 23, 10, 1, 21, 384052, tzinfo=datetime.timezone(datetime.timedelta(0), '+00:00')), 'progress_msg': None}
>>> vault.cook_request(obj_type="directory", obj_id=hash_to_bytes("838bc7ca7541fd96ed6e665c1485a0f8b6d4c3f3"))
{'id': 1, 'type': 'directory', 'object_id': b'\x83\x8b\xc7\xcauA\xfd\x96\xednf\\\x14\x85\xa0\xf8\xb6\xd4\xc3\xf3', 'task_id': None, 'task_status': 'new', 'sticky': False, 'ts_created': datetime.datetime(2020, 10, 23, 10, 1, 21, 384052, tzinfo=datetime.timezone(datetime.timedelta(0), '+00:00')), 'ts_done': None, 'ts_last_access': datetime.datetime(2020, 10, 23, 10, 1, 21, 384052, tzinfo=datetime.timezone(datetime.timedelta(0), '+00:00')), 'progress_msg': None}
...

server code:

$ doco logs -f swh-vault
...
swh-vault_1                     | INFO:aiohttp.access:192.168.176.30 [23/Oct/2020:10:04:30 +0000] "GET /fetch HTTP/1.1" 400 1132 "-" "python-requests/2.24.0"
swh-vault_1                     | [INFO] aiohttp.access -- 192.168.176.30 [23/Oct/2020:10:04:30 +0000] "GET /fetch HTTP/1.1" 400 1132 "-" "python-requests/2.24.0"
swh-vault_1                     | INFO:aiohttp.access:192.168.176.30 [23/Oct/2020:10:12:55 +0000] "GET /progress HTTP/1.1" 200 394 "-" "python-requests/2.24.0"
swh-vault_1                     | [INFO] aiohttp.access -- 192.168.176.30 [23/Oct/2020:10:12:55 +0000] "GET /progress HTTP/1.1" 200 394 "-" "python-requests/2.24.0"
swh-vault_1                     | INFO:aiohttp.access:192.168.176.30 [23/Oct/2020:10:12:55 +0000] "POST /cook HTTP/1.1" 200 394 "-" "python-requests/2.24.0"
swh-vault_1                     | [INFO] aiohttp.access -- 192.168.176.30 [23/Oct/2020:10:12:55 +0000] "POST /cook HTTP/1.1" 200 394 "-" "python-requests/2.24.0"
...

Diff Detail

Repository

rDCORE Foundations and core functionalities

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 16595
Build 25578: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 25577: arc lint + arc unit

Event Timeline

ardumont created this revision.Oct 23 2020, 12:14 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptOct 23 2020, 12:14 PM

Build is green

Patch application report for D4341 (id=15369)

Rebasing onto 0796ac6c6c...

Current branch diff-target is up to date.

Changes applied before test

commit 0011c7327d1812ae969223796963209b196136b7
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 23 12:06:46 2020 +0200

    rpc_client: Allow http method declaration consistently with the rpc server
    
    a7d1aa7 introduced that use for the server. Without doing this consistenly in
    the rpc client part, this won't work though.
    
    Related to a7d1aa7

See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/115/ for more details.

Harbormaster completed remote builds in B16594: Diff 15369.Oct 23 2020, 12:15 PM

Revert comment change

Build is green

Patch application report for D4341 (id=15370)

Rebasing onto 0796ac6c6c...

Current branch diff-target is up to date.

Changes applied before test

commit 15fa5ffa4fd1d3267f50ea7846f64ccd0ab65d63
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 23 12:06:46 2020 +0200

    rpc_client: Allow http method declaration consistently with the rpc server
    
    a7d1aa7 introduced that use for the server. Without doing this consistenly in
    the rpc client part, this won't work though.
    
    Related to a7d1aa7

See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/116/ for more details.

Harbormaster completed remote builds in B16595: Diff 15370.Oct 23 2020, 12:19 PM

wait, what? It would be simpler to just use POST everywhere, we just need to change the vault's protocol

ardumont added inline comments.Oct 23 2020, 12:26 PM

swh/core/api/__init__.py
308	i've simplified the code in D4342

wait, what? It would be simpler to just use POST everywhere, we just need to change the vault's protocol

yes, it crossed my mind as well.

I'm currently acting on my remark D4329#inline-29651

wait, what? It would be simpler to just use POST everywhere, we just need to change the vault's protocol

yes, it crossed my mind as well.

I'm currently acting on my remark D4329#inline-29651

and also to try and minimize existing behavior change for vault clients like the webapp...

In D4341#108138, @ardumont wrote:

yes, it crossed my mind as well.

I'm currently acting on my remark D4329#inline-29651

and also to try and minimize existing behavior change for vault clients like the webapp...

Let's cross check with @anlambert ;)

@anlambert Do you foresee impact if i'm changing the vault rpc client/server methods from get to post?
(I think there are but it's not clear yet)

Note that, currently, i'm holding back D4329.

Within docker, rpc client/server code is working fine in and on itself (see the
paste in the test plan).

But the webapp is not happy anyways. I don't have logs, just a popup mentioning
"Archive cooking service is currently experiencing issues. Please try again
later."

I don't find more proper logs about the related issue.

I'm digging in the webapp code right now to understand where i'm falling short.
I gather it's some sort of query build by the webapp somehow but i'm unsure.

Any hints?

In D4341#108154, @ardumont wrote:

In D4341#108138, @ardumont wrote:

yes, it crossed my mind as well.

I'm currently acting on my remark D4329#inline-29651

and also to try and minimize existing behavior change for vault clients like the webapp...

Let's cross check with @anlambert ;)

@anlambert Do you foresee impact if i'm changing the vault rpc client/server methods from get to post?
(I think there are but it's not clear yet)

Note that, currently, i'm holding back D4329.

Within docker, rpc client/server code is working fine in and on itself (see the
paste in the test plan).

But the webapp is not happy anyways. I don't have logs, just a popup mentioning
"Archive cooking service is currently experiencing issues. Please try again
later."

I don't find more proper logs about the related issue.

I'm digging in the webapp code right now to understand where i'm falling short.
I gather it's some sort of query build by the webapp somehow but i'm unsure.

Any hints?

ok, i found my issue.
D4392 renamed rpc methods to decrease the diff's size but that created divergence in the rpc vault client part... which creates issues down the line.
I'll realign properly those methods.

And drop this diff \m/ and the next one ;)

Cheers,

ardumont abandoned this revision.Oct 23 2020, 3:10 PM