Page MenuHomePhabricator
Create Task
Maniphest T389819

Remove deprecated parameters from ServerSideAccountCreation
Open, Needs TriagePublic
Actions

Description

CampaignsSecondaryAuthenticationProvider.php has these still:

			// @todo: Remove these unused fields when they're no longer required by the schema.
			'token' => '',
			'userBuckets' => '',

They were deprecated in rECAMf5bdc9e4df8f: Update extension for AuthManager since May 2016.

https://gitlab.wikimedia.org/repos/data-engineering/schemas-event-secondary/-/blob/master/jsonschema/analytics/legacy/serversideaccountcreation/1.3.0.yaml

Why are they still marked under required?

Can we remove them from required as a step before bumping the schema? Or does default handling not happen?

Do we need to bump the major version (2.x)? Or can I do it as part of T379179: Send hCaptcha API response data to event platform and mark it as 1.4.0 too?

Details

SubjectRepoBranchLines +/-
CampaignsSecondaryAuthenticationProvider: Remove unused event parametersmediawiki/extensions/Campaignsmaster+0 -3
Customize query in gerrit

Related Objects

Event Timeline

Reedy created this task.Mon, Mar 24, 3:20 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMon, Mar 24, 3:20 PM
Reedy updated the task description. (Show Details)Mon, Mar 24, 3:21 PM
gerritbot added a comment.Mon, Mar 24, 3:39 PM

Change #1130640 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Campaigns@master] CampaignsSecondaryAuthenticationProvider: Remove unused event parameters

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

gerritbot added a project: Patch-For-Review.Mon, Mar 24, 3:39 PM
Reedy added a project: Event-Platform.Mon, Mar 24, 3:58 PM

https://wikitech.wikimedia.org/wiki/Event_Platform/Schemas/Guidelines#Backwards_compatible_modifications_only

https://wikitech.wikimedia.org/wiki/Event_Platform/Schemas/Guidelines#Major_schema_version_upgrades

It is technically allowed to make backwards incompatible schema changes between major version changes. However, the tooling does not support it. Doing so will require manual migrations for any producers and consumers (including downstream Hive tables) of streams that use the schema.

So yeah, it'd need to be version 2.0.0... Which is fine.

This does seem worth doing

Reedy mentioned this in T389822: ServerSideAccountCreation violates Identifier Naming Rules?.Mon, Mar 24, 4:11 PM
Reedy updated the task description. (Show Details)
Maintenance_bot added a project: Data-Engineering.Mon, Mar 24, 4:30 PM
Ottomata subscribed.Tue, Mar 25, 5:54 PM

@Reedy how can we help?

Ottomata moved this task from Incoming (new tickets) to Needs Clarification on the Data-Engineering board.Tue, Mar 25, 5:54 PM
Reedy added a comment.Tue, Mar 25, 5:57 PM

Just wondering what is the process for us to do this removal of the deprecated parameters.

They've been unused for ~9 years..

Is anything needed server side to help "deploy" it etc?

Reedy added a comment.Tue, Mar 25, 6:16 PM

Or, as the answer was in T389822: ServerSideAccountCreation violates Identifier Naming Rules? from yourself...

In T389822#10670119, @Ottomata wrote:

Or... you could make a brand new Metrics Platform based instrumentation and decom the ServerSideAccountCreation one ;)

And just do a clean break.

Ottomata added a comment.Tue, Mar 25, 7:23 PM

Or... you could make a brand new Metrics Platform based instrumentation

oh yes, that would be best!

Just wondering what is the process for us to do this removal of the deprecated parameters.

The easiest thing to do would be to update the documentation for these fields and note them as deprecated.

and/or, I believe you could make a new major version, and remove them from the list of required fields.
Removing the actual field properties may break things, but removing them from the list of required fields will not break Hive, and I'd assume that Hive is the only downstream consumer of this stream.

If they are not required by the event schema, then you can stop setting them in CampaignsSecondaryAuthenticationProvider.php.

Ottomata moved this task from Needs Clarification to Tag with Radar on the Data-Engineering board.Tue, Apr 1, 5:32 PM
Ottomata added a project: Data-Engineering-Radar.
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits