Page MenuHomePhabricator

Add lat, lon, image, monument_article and registrant_url to each source
Closed, InvalidPublic

Description

Every source should have the fields lat, lon, image, monument_article and registrant_url even if it's not available. This is the case for most sources, but not all of them.


Version: unspecified
Severity: normal

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 1:54 AM
bzimport set Reference to bz53813.
bzimport added a subscriber: Unknown Object (MLST).

@Multichill : Is that still needed ?

I think so. Shouldn't be too hard to do.

Lokal_Profil subscribed.

To clarify. We are talking about the monuments_xx_(xx) tables here. Rather than monuments_all?

To clarify. We are talking about the monuments_xx_(xx) tables here. Rather than monuments_all?

Think so!

Once https://gerrit.wikimedia.org/r/#/c/290557/ has been merged it should be easy to add these fields to the required list and then patch any missing sources.

Also to clarify: dest should match lat, lon, image, monument_article and registrant_url rather than source (allowing these fields different names on each wiki). This might mean renaming some existing fields and if so also updating fill_table_monuments_all.sql.

Ok after I started working on this I suddenly questioned why it is needed.

There are three cases here:

  1. Ensure the variables are present in monuments_config source fields for each dataset.
  2. Ensure the variables are present in monuments_config dest fields for each dataset.
  3. Ensure the variables are present in fill_monuments_all for each dataset.

1.) Has the benefit that it would allow us to write data to the wiki-lists. The downside is that we would either have to support aliases or force the local wikis to use our parameter names. So I'll conclude that this is neither intended or desired.

2.) Has the benefit that these variables will be present in the monuments_country_(lang) tables (with a standardised name as a bonus) and it makes it easier to figure out which fields to use in fill_table_monuments_all.sql. However we only ever use these tables to populate fill_table_monuments_all.sql where the mapping needs to be done regardless. Another downside is that we end up mapping from non-existent template parameters. Feels like a very small gain for the effort of finding every dest that uses a localised name.

3.) This ensures that the variables can always be replaced into monuments_all_tmp. If they are not present in monuments_config we simply map '' or NULL to it. But then we already set defaults for monuments_all_tmp during creation so what is the need? If anything this causes inconsistencies since we are inconsistent in using '' or NULL (with respect to the initial defaults).

@JeanFred, @Multichill: Am I missing some obvious use case for doing this?

https://gerrit.wikimedia.org/r/#/c/292532/ introduces the basic method by which to enforce 3.) if so desired.

@JeanFred, @Multichill: Am I missing some obvious use case for doing this?

Found my local branch looking at this but not sure which direction (if any) I should take it.

Change 309859 had a related patch set uploaded (by Lokal Profil):
[Not READY] Require lat, lon, image, monument_article, registrant_url

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

Change 309862 had a related patch set uploaded (by Lokal Profil):
[NOT READY]Require lat, lon, image, monument_article and registrant_url

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

I put up my two local branches for case 2 and 3 (in T55813#2352533)
note that only one of them should be used (if any).

I'll leave them as is until I get feedback on how we want to proceed in this.

Lokal_Profil changed the task status from Open to Stalled.Oct 2 2016, 9:40 PM

Closing this as invalid and abandoning both patchsets (everything is still there if someone wants to pick it up but it's slowly going out of sync with the rest of the repo)

Change 309862 abandoned by Lokal Profil:
[NOT READY]Require lat, lon, image, monument_article and registrant_url

Reason:
Abandoning and closing T55813 as invalid

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

Change 309859 abandoned by Lokal Profil:
[Not READY] Require lat, lon, image, monument_article, registrant_url

Reason:
Abandoning and closing T55813 as invalid

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