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

Expose new end-point on /getAll on SettingsResource v2 for ETL stuff #893

Open
rehammuzzamil opened this issue Jul 12, 2021 · 8 comments · May be fixed by #894
Open

Expose new end-point on /getAll on SettingsResource v2 for ETL stuff #893

rehammuzzamil opened this issue Jul 12, 2021 · 8 comments · May be fixed by #894
Assignees
Labels
enhancement New feature or request

Comments

@rehammuzzamil
Copy link
Contributor

rehammuzzamil commented Jul 12, 2021

End-point url : /getAll
Request Params : Same as /rest/v2/settings/

What is missing in GET /rest/v2/settings/ end-point?
ETL Team wants the data in the monotonic order of metadata_version, but the API fetches the data from the database, and from the implementation perspective all the settings metadata are re-arranged in a map of settings configurations where the key is the settings identifier. Here is an example:

image

Why should we create a new end-point?
The idea of exposing the new end-point getAll is because it has been the convention on all other Resources/Controllers for ETL stuff. This new end-point will hold a custom implementation in which we won't be clubbing the settings metadata against a particular identifier, rather fetch all the records from the settings_metadata table that meets the criteria and then return as is in a list in increasing order of metadata_version.

Implementation details
We will add an isETL flag in the SettingSearchBean and set it as true in the/getAllend-point. Repository method
List<SettingConfiguration> findSettings(SettingSearchBean settingQueryBean, int limit, Map<String, TreeNode<String, Location>> treeNodeHashMap) implementation is modified and handled if isETL flag is found to be true.

More details can be found here.

cc : @samkanga @dubdabasoduba @bennsimon @Wambere

@Wambere
Copy link

Wambere commented Jul 13, 2021

@rehammuzzamil from my comment here it seemed that the serverVersions were not getting updated in batches as initially though, but one at a time based on what exactly was changed. So maybe it's possible to have this endpoint filtered and ordered by the serverVersion instead of the metadataVersion?

@rehammuzzamil
Copy link
Contributor Author

I have shared my findings here.
Please note work on adding a new column metadata_version is already a part of Master.
@dubdabasoduba @bennsimon your thoughts on this?
cc : @Wambere @samkanga

@bennsimon
Copy link
Member

@Wambere Changing the behaviour of the serverVersion will affect v1 fetch endpoints e.g this .I would suggest we proceed with metadataVersion.

cc: @dubdabasoduba @rehammuzzamil

@Wambere
Copy link

Wambere commented Jul 14, 2021

@bennsimon I am not requesting a change in behaviour, simply an order by serverVersion from this new endpoint, so that it works like all other /getAll endpoints.

Just to clarify and to make sure that I am not missing anything

  1. My initial understanding was that the settings are currently saved in batches (grouped by identifier) and when one record in the batch is changed/updated, all records in the batch get new serverVersions
  2. According to my testing, the above assumption is incorrect. Looking at the records from our test server below (only showing key values)
[
    { "key": "28713d21-d4f9-49b7-aab7-b07838fb086f", "settingsId": "1", "settingMetadataId": "1", "metadataVersion": 1, "serverVersion": 1},
    { "key": "65e39c5b-801d-46fc-ba26-7840cb3eb781", "settingsId": "1", "settingMetadataId": "3", "metadataVersion": 3, "serverVersion": 1},
    { "key": "6cb96ec5-a89f-4eb6-bb5e-83692687095e", "settingsId": "1", "settingMetadataId": "4", "metadataVersion": 4, "serverVersion": 1},
    { "key": "2ac37ac3-3d78-4642-9e8b-49058a8175d0", "settingsId": "1", "settingMetadataId": "5", "metadataVersion": 5, "serverVersion": 1},
    { "key": "03153b41-0a1b-411a-ba0f-a6cd247898d4", "settingsId": "1", "settingMetadataId": "2", "metadataVersion": 28, "serverVersion": 15},
    { "key": "ecec3049-78fc-4b9c-b6e1-9a25ad43655f", "settingsId": "2", "settingMetadataId": "6", "metadataVersion": 6, "serverVersion": 2 },
    { "key": "0af09fba-4544-4703-bfc6-42366f9dbc3d", "settingsId": "2", "settingMetadataId": "7", "metadataVersion": 7, "serverVersion": 2},
    { "key": "49aff650-e9c6-4ce5-8898-64087749fde9", "settingsId": "2", "settingMetadataId": "8", "metadataVersion": 8, "serverVersion": 2},
    { "key": "2ac37ac3-3d78-4642-9e8b-49058a8175d0", "settingsId": "2", "settingMetadataId": "10", "metadataVersion": 10, "serverVersion": 2},
    { "key": "5b27141a-1478-45ce-9f27-85c4fb338cdd", "settingsId": "2", "settingMetadataId": "9", "metadataVersion": 29, "serverVersion": 16},
    { "key": "e42f4351-d7fb-43e6-8ebf-a6b78a980f1f", "settingsId": "3", "settingMetadataId": "11", "metadataVersion": 11, "serverVersion": 3},
    { "key": "c7b8c339-14d8-4fd7-9900-cdaf42d360a1", "settingsId": "3", "settingMetadataId": "12", "metadataVersion": 12, "serverVersion": 3},
    { "key": "45b52242-9d05-4f33-b302-9063b32edf2f", "settingsId": "4", "settingMetadataId": "13", "metadataVersion": 14, "serverVersion": 5},
    { "key": "d524f46e-78e0-479f-bdf2-ea9c370ffd57", "settingsId": "5", "settingMetadataId": "14", "metadataVersion": 15, "serverVersion": 6},
    { "key": "d524f46e-78e0-479f-bdf2-ea9c370ffd57", "settingsId": "6", "settingMetadataId": "15", "metadataVersion": 16, "serverVersion": 7},
    { "key": "8e4d5126-7dd5-4cfd-91a8-f858141dfa0f", "settingsId": "6", "settingMetadataId": "16", "metadataVersion": 17, "serverVersion": 7},
    { "key": "4dcbb6b4-f864-468f-b21c-292a0c664f63", "settingsId": "6", "settingMetadataId": "18", "metadataVersion": 19, "serverVersion": 7},
    { "key": "7dad8abe-d4d2-447d-a8a1-8a7475167ecd", "settingsId": "6", "settingMetadataId": "17", "metadataVersion": 30, "serverVersion": 17},
    { "key": "d524f46e-78e0-479f-bdf2-ea9c370ffd57", "settingsId": "7", "settingMetadataId": "19", "metadataVersion": 20, "serverVersion": 8},
    { "key": "d524f46e-78e0-479f-bdf2-ea9c370ffd57", "settingsId": "8", "settingMetadataId": "20", "metadataVersion": 21, "serverVersion": 9},
    { "key": "a524f46e-78e0-479f-bdf2-ea9c370ffd57", "settingsId": "9", "settingMetadataId": "21", "metadataVersion": 22, "serverVersion": 10},
    { "key": "a524f46e-78e0-479f-bdf2-ea9c370ffd57", "settingsId": "10", "settingMetadataId": "22", "metadataVersion": 23, "serverVersion": 11},
    { "key": "630e981d-e61b-4eb7-a4fa-c20d06e77ccb", "settingsId": "10", "settingMetadataId": "23", "metadataVersion": 24, "serverVersion": 11},
    { "key": "a524f46e-78e0-479f-bdf2-ea9c370ffd57", "settingsId": "11", "settingMetadataId": "24", "metadataVersion": 25, "serverVersion": 12},
    { "key": "a524f46e-78e0-479f-bdf2-ea9c370ffd57", "settingsId": "12", "settingMetadataId": "25", "metadataVersion": 26, "serverVersion": 13},
    { "key": "812ddd15-391f-4db4-b727-017745bc0216", "settingsId": "13", "settingMetadataId": "26", "metadataVersion": 27, "serverVersion": 14}
]

I edited key 03153b41-0a1b-411a-ba0f-a6cd247898d4 the settingsId remained 1, the settingMetadataId remained 2, the metadataVersion changed to 28 and the serverVersion changed to 15, however all the other records in group remained with serverVersion 1.
I edited key 5b27141a-1478-45ce-9f27-85c4fb338cdd the settingsId remained 2, the settingMetadataId remained 9, the metadataVersion changed to 29 and the serverVersion changed to 16, however all the other records in group remained with serverVersion 2.
I edited key 7dad8abe-d4d2-447d-a8a1-8a7475167ecd the settingsId remained 6, the settingMetadataId remained 17, the metadataVersion changed to 30 and the serverVersion changed to 17, however all the other records in group remained with serverVersion 7.

  1. My request is to have this response ordered by serverVersion

Does this make sense? Either this is true, or there is some bug in our test server such that all serverVersions are not changing as expected?

I don't mind proceeding with using the metadataVersion to order, I'm just trying to understand the reasoning and why this is not working as expected or if there is something else that I am not seeing

@bennsimon
Copy link
Member

bennsimon commented Jul 14, 2021

@Wambere serverVersions are meant to be sequences they should not be duplicated since its used for pagination so it would return duplicate records on the endpoints in case the number of settings metadata with same serverVersion exceed the page size. Referring to the new endpoint.

On the app we fetch using endpoints mapping settings to settingsMetadata so we wouldnt have the same scenario mentioned above.

Or you can work with {{url}}/opensrp/rest/v2/settings/getAll?metadataVersion=0&orderByType=ASC&orderByFieldName=server_version .. here i have switched the role of server_version and metadata_version.

@rehammuzzamil
Copy link
Contributor Author

@Wambere Can you please confirm is this okay for the ETL team to use a new end-point/getAllwith the approach mentioned above by @bennsimon?
Thanks.

@Wambere
Copy link

Wambere commented Jul 27, 2021

It is still not clear to me why this needs to be different. A meeting would be nice to clear things up and make sure we are on the same page

@ageryck
Copy link

ageryck commented Jul 29, 2021

Hey all setting up a meeting to have us iron out this to be all on the same page, will share an invite to your calendars

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants