Page MenuHomeSoftware Heritage
Differential D1670

api: rename several API related classes
ClosedPublic
Actions

Authored by douardda on Jul 1 2019, 12:17 PM.

Details

Reviewers
vlorentz
ardumont
Group Reviewers
Reviewers
Commits
rDCORE09053ca741fb: tests: apply api class renamings to tests
rDCOREf4125dbd72e6: api: rename several API related classes
Summary

to get rid of the SWH prefix and five more consistent names, especially
the swh.api.asynchonous.SWHRemoteAPI class which is very confusing.

Deprecate old class names using the Deprecate python module.

Also (second revision) update tests accordingly.

Diff Detail

Repository
rDCORE Foundations and core functionalities
Branch
content-neg
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 6591
Build 9193: tox-on-jenkinsJenkins
Build 9192: arc lint + arc unit

Event Timeline

douardda created this revision.Jul 1 2019, 12:17 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJul 1 2019, 12:17 PM
swh-public-ci added a comment.Jul 1 2019, 12:24 PM

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/233/ for more details.

Harbormaster completed remote builds in B6591: Diff 5585.Jul 1 2019, 12:24 PM
douardda added a child revision: D1671: api/async: fix json decoder and add a few tests for the asynchronous api code.Jul 1 2019, 1:42 PM
ardumont added a subscriber: ardumont.Jul 1 2019, 2:06 PM
ardumont added inline comments.
swh/core/api/asynchronous.py
52

Can we please drop the SWH prefix?

I try to remove that bit by bit everywhere i can.
The module already specifies we are within the swh environment, so no need to be extra redundant.

douardda added inline comments.Jul 1 2019, 2:13 PM
swh/core/api/asynchronous.py
52

ok but then I need to drop it also in the sync version of the code...

douardda updated this revision to Diff 5589.Jul 1 2019, 3:05 PM

Rename several API classes

The first git revision does the renamings in the code:

  • sync:
    • SWHServerAPIApp => RPCServerApp
    • MetaSWHRemoteAPI => MetaRPCClient
    • SWHRemoteAPI => RPCClient
  • async:
    • SWHRemoteAPI => RPCServerApp (!)

The second revision updates the tests accordingly

Tests pass OK before and after this second revision.

douardda retitled this revision from api/async: rename SWHRemoteAPI as SWHServerAPIApp to api: rename several API related classes.Jul 1 2019, 3:06 PM
douardda edited the summary of this revision. (Show Details)
vlorentz added a subscriber: vlorentz.EditedJul 1 2019, 3:10 PM

I don't think we should add a dependency just for that. Raising a warning directly (like swh-storage already does) would work

swh-public-ci added a comment.Jul 1 2019, 3:11 PM

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/236/ for more details.

Harbormaster completed remote builds in B6597: Diff 5589.Jul 1 2019, 3:11 PM
douardda added a comment.Jul 1 2019, 3:29 PM
In D1670#38521, @vlorentz wrote:

I don't think we should add a dependency just for that. Raising a warning directly (like swh-storage already does) would work

I think we should have such a facility in swh.core and use it everywhere consistently. Using plain DeprecationWarning is a pain in the neck, does not encourage you to declare which version adds the deprecation, and so on. This goes towards a more declarative deprecation handling, which is better.

vlorentz accepted this revision.Jul 1 2019, 4:23 PM
This revision is now accepted and ready to land.Jul 1 2019, 4:23 PM
vlorentz added a comment.Jul 1 2019, 4:23 PM

"ok then"

ardumont accepted this revision.Jul 1 2019, 4:31 PM
douardda updated this revision to Diff 5664.Jul 4 2019, 12:10 PM
douardda edited the summary of this revision. (Show Details)

typo in commit msg

swh-public-ci added a comment.Jul 4 2019, 12:12 PM

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/239/ for more details.

Harbormaster completed remote builds in B6674: Diff 5664.Jul 4 2019, 12:12 PM
Closed by commit rDCOREf4125dbd72e6: api: rename several API related classes (authored by douardda). · Explain WhyJul 4 2019, 12:13 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
2 lines
swh/
core/
api/
8 lines

Diff 5585

View Options

requirements.txt

deprecation
PyYAML PyYAML
systemd-python systemd-python
View Options

swh/core/api/asynchronous.py

import json import json
import logging import logging
import pickle import pickle
import sys import sys
import traceback import traceback
import aiohttp.web import aiohttp.web
import deprecation
import multidict import multidict
from .serializers import msgpack_dumps, msgpack_loads, SWHJSONDecoder from .serializers import msgpack_dumps, msgpack_loads, SWHJSONDecoder
def encode_data_server(data, **kwargs): def encode_data_server(data, **kwargs):
return aiohttp.web.Response( return aiohttp.web.Response(
body=msgpack_dumps(data), body=msgpack_dumps(data),
Show All 27 Lines async def middleware_handler(request):
logging.exception(e) logging.exception(e)
exception = traceback.format_exception(*sys.exc_info()) exception = traceback.format_exception(*sys.exc_info())
res = {'exception': exception, res = {'exception': exception,
'exception_pickled': pickle.dumps(e)} 'exception_pickled': pickle.dumps(e)}
return encode_data_server(res, status=500) return encode_data_server(res, status=500)
return middleware_handler return middleware_handler
class SWHRemoteAPI(aiohttp.web.Application): class SWHServerAPIApp(aiohttp.web.Application):
ardumontUnsubmitted
Not Done
Inline Actions

Can we please drop the SWH prefix?

I try to remove that bit by bit everywhere i can.
The module already specifies we are within the swh environment, so no need to be extra redundant.

ardumont: Can we please drop the SWH prefix? I try to remove that bit by bit everywhere i can. The…
douarddaAuthorUnsubmitted
Done
Inline Actions

ok but then I need to drop it also in the sync version of the code...

douardda: ok but then I need to drop it also in the sync version of the code...
def __init__(self, *args, middlewares=(), **kwargs): def __init__(self, *args, middlewares=(), **kwargs):
middlewares = (error_middleware,) + middlewares middlewares = (error_middleware,) + middlewares
super().__init__(*args, middlewares=middlewares, **kwargs) super().__init__(*args, middlewares=middlewares, **kwargs)
SWHRemoteAPI = deprecation.deprecated(
deprecated_in='0.0.64',
details='Use the SWHServerAPIApp instead')(SWHServerAPIApp)
About · Developer information · Legal