Page MenuHomePhabricator

Replace use of deprecated methods (mainly in ApiErrorReporter)
Closed, ResolvedPublic

Description

rMW4e6810e4a2c1: API: i18n for warnings and errors deprecated several things in the API. The relevant deprecations here are:

  • ApiBase::dieUsage() replaced with ApiBase::dieWithError()
  • ApiBase::setWarning() replaced with ApiBase::addWarning()
  • ApiBase::getErrorFromStatus() replaced with ApiErrorFormatter::arrayFromStatus()
  • UsageException replaced with ApiUsageException (Gerrit change 327545 seems to be working on this one)

Gerrit change 321464 has some more details on possible replacements for the use of the ApiBase methods in your ApiErrorReporter class. The interface being added in Gerrit change 325984 might also be handy.

Event Timeline

Anomie created this task.Dec 15 2016, 9:25 PM
Restricted Application added a project: Wikidata. · View Herald TranscriptDec 15 2016, 9:25 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 321464 had a related patch set uploaded (by Anomie):
WIP: Update for API error i18n

https://gerrit.wikimedia.org/r/321464

Change 327545 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Replace all deprecated UsageException in Repo with ApiUsageException

https://gerrit.wikimedia.org/r/327545

thiemowmde triaged this task as Low priority.
thiemowmde moved this task from incoming to in progress on the Wikidata board.
thiemowmde moved this task from Proposed to Review on the Wikidata-Former-Sprint-Board board.

Change 327704 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Replace deprecated methods in ApiErrorReporter

https://gerrit.wikimedia.org/r/327704

Change 327704 abandoned by Thiemo Mättig (WMDE):
Replace deprecated methods in ApiErrorReporter

Reason:
If you believe that a first small, incomplete step forward deserves a -1 just for being what it is, I'm wasting my time. Feel free to continue working on this.

https://gerrit.wikimedia.org/r/327704

Change 328151 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Inject ApiBase into ItemByTitleHelper for proper error reporting

https://gerrit.wikimedia.org/r/328151

Change 328153 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Remove ApiUsageException related dead code from EntityLoadingHelperTest

https://gerrit.wikimedia.org/r/328153

Change 327545 merged by jenkins-bot:
Replace all deprecated UsageException in Repo with ApiUsageException

https://gerrit.wikimedia.org/r/327545

Change 328151 merged by Daniel Kinzler:
Inject ApiBase into ItemByTitleHelper for proper error reporting

https://gerrit.wikimedia.org/r/328151

Change 328153 merged by jenkins-bot:
Remove ApiUsageException related dead code from EntityLoadingHelperTest

https://gerrit.wikimedia.org/r/328153

What still needs to be done here?

What still needs to be done here?

Updating your ApiErrorReporter class to not use the deprecated methods. There was some relevant discussion on https://gerrit.wikimedia.org/r/#/c/327704/ after it was abandoned.

daniel raised the priority of this task from Low to Normal.Jun 19 2017, 10:28 AM

Change 360872 had a related patch set uploaded (by Thiemo Mättig (WMDE); owner: Thiemo Mättig (WMDE)):
[mediawiki/extensions/Wikibase@master] Minor refactorings and cleanups in ApiErrorReporter and tests

https://gerrit.wikimedia.org/r/360872

Change 353545 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Don't use deprecated methods in ApiErrorReporter.

https://gerrit.wikimedia.org/r/353545

Change 321464 abandoned by Anomie:
WIP: Update for API error i18n

Reason:
Done in Ib5b1eae2ab66e633a438d2705d7b96f386f16939

https://gerrit.wikimedia.org/r/321464

Change 360872 abandoned by Thiemo Mättig (WMDE):
Minor refactorings and cleanups in ApiErrorReporter and tests

Reason:
This became obsolete because Ib5b1eae got merged.

https://gerrit.wikimedia.org/r/360872

WMDE-leszek closed this task as Resolved.Jun 23 2017, 7:53 AM
WMDE-leszek reopened this task as Open.
WMDE-leszek moved this task from Review to Done on the Wikidata-Former-Sprint-Board board.
WMDE-leszek added a subscriber: WMDE-leszek.

I might have marked this as resolved too early. Can we call this done @daniel @thiemowmde ?

aude added a comment.EditedJun 26 2017, 1:13 AM

We have lost some of the error descriptions.

currently on Wikidata:

https://www.wikidata.org/w/api.php?action=wbgetentities

{
    "error": {
        "code": "param-missing",
        "info": "A parameter that is required was missing. (Either provide the item \"ids\" or pairs of \"sites\" and \"titles\" for corresponding pages)",
        "messages": [
            {
                "name": "wikibase-api-param-missing",
                "parameters": [],
                "html": {
                    "*": "A parameter that is required was missing."
                }
            }
        ],
        "*": "See https://www.wikidata.org/w/api.php for API usage. Subscribe to the mediawiki-api-announce mailing list at <https://lists.wikimedia.org/mailman/listinfo/mediawiki-api-announce> for notice of API deprecations and breaking changes."
    },
    "servedby": "mw1278"
}

Wikibase master:

{
    "error": {
        "code": "param-missing",
        "info": "A parameter that is required was missing.",
        "messages": [
            {
                "name": "wikibase-api-param-missing",
                "parameters": [],
                "html": {
                    "*": "A parameter that is required was missing."
                }
            }
        ],
        "*": "See https://wikidatawiki/w/api.php for API usage. Subscribe to the mediawiki-api-announce mailing list at <https://lists.wikimedia.org/mailman/listinfo/mediawiki-api-announce> for notice of API deprecations and breaking changes."
    }
}

Think we want to make the full api error messages (including descriptions) translatable and add i18n messages for them.

The way ApiErrorReporter works now, the messages are based on the error code. (e.g. wikibase-api-param-missing). This error code is shared by numerous different api modules, each with a different error description.

We should still share the error code and not change it (api users might depend on how it is now), but allow the message / description to differ and be translatable.

thiemowmde closed this task as Resolved.Jun 26 2017, 8:53 AM
thiemowmde reassigned this task from thiemowmde to daniel.
thiemowmde removed a project: Patch-For-Review.

I'm closing this for now because the Wikibase code base does not contain calls to the methods and classes listed in this tickets description any more.

@aude, I'm working on these right now, see https://gerrit.wikimedia.org/r/360871.