Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update required media item fields #1000

Merged
merged 2 commits into from
Jun 6, 2018

Conversation

mdholloway
Copy link
Contributor

As of https://gerrit.wikimedia.org/r/#/c/436360/ (adding support for
Mathoid images) media items may not have the usual set of Commons
metadata. This updates the media spec to fix the monitoring check.

As of https://gerrit.wikimedia.org/r/#/c/436360/ (adding support for
Mathoid images) media items may not have the usual set of Commons
metadata.  This updates the media spec to fix the monitoring check.
@Pchelolo
Copy link
Contributor

Pchelolo commented Jun 5, 2018

Congrats @mdholloway - you've made PR #1000 :)

@mdholloway
Copy link
Contributor Author

\o/

@Pchelolo
Copy link
Contributor

Pchelolo commented Jun 5, 2018

1 test is still failing for the media endpoint.

@d00rman
Copy link
Contributor

d00rman commented Jun 5, 2018

@mdholloway It would also be good for RB's and MCS' specs to have the same x-maples for the same end points, so that when things start failing we can identify the source more easily. Right now RB is alerting because of the change in MCS, but MCS is not.

@Pchelolo
Copy link
Contributor

Pchelolo commented Jun 5, 2018

@mdholloway test is still failing with body item original not found in the response

To actually fix the monitoring check.
@mdholloway
Copy link
Contributor Author

@mdholloway It would also be good for RB's and MCS' specs to have the same x-maples for the same end points, so that when things start failing we can identify the source more easily. Right now RB is alerting because of the change in MCS, but MCS is not.

See https://gerrit.wikimedia.org/r/#/c/437636/. Small change to follow in restbase as well.

mdholloway added a commit to mdholloway/restbase that referenced this pull request Jun 5, 2018
Follow-up from wikimedia#1000 (comment).

Reconciles this check with mobileapps to aid in diagnosing alerts.

See also https://gerrit.wikimedia.org/r/#/c/437636/ on the mobileapps side.
@mdholloway mdholloway mentioned this pull request Jun 5, 2018
@Pchelolo Pchelolo merged commit 0d788db into wikimedia:master Jun 6, 2018
- type
- thumbnail
- license
- original
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is something wrong here @mdholloway : the spec test requires section_id and type to be present, but the definition additionally requires the original field. These need to match.

@mdholloway mdholloway deleted the required-media-items branch June 6, 2018 14:45
wmfgerrit pushed a commit to wikimedia/mediawiki-services-mobileapps that referenced this pull request Jun 6, 2018
As pointed out by mobrovac[1], this will make it easier to diagnose alerts.

[1] wikimedia/restbase#1000 (comment)

Change-Id: I6d3e951a72cbfababef22195aa0d73d211ddca9b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants