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

fix: bw-incompatible update to user elasticsearch index #1241

Merged
merged 2 commits into from
Jun 16, 2021

Conversation

dkunitsk
Copy link

@dkunitsk dkunitsk commented Jun 16, 2021

Signed-off-by: Dmitriy Kunitskiy dkunitskiy@lyft.com

Summary of Changes

This change fixes the discrepancy between the user ES index located in common vs databuilder. The discrepancy is in one field: full_name vs name.

This is being done in preparation for deleting the elasticsearch_constants.py databuilder file and importing these mappings from common instead.

The same cleanup was performed here but had to be reverted because of some marshmallow thing that's no longer relevant.

Why this change is safe-ish:

  1. the ES document for user has full_name.
  2. search service uses full_name: model, proxy
  3. only usage I could find of the old amundsen-common's index is here in the ES proxy. This function is only used to create an index when an index is not found. But the ES publisher does create an index as its first step so in theory this function should be mostly unused.

In other words, data should be loaded using full_name and searched using full_name. The fact that an index exists in another place with name instead of full_name just means people who've been using it were probably having semi-broken user search.

Notice to community

Backwards incompatible change alert!
We're deleting databuilder's elasticsearch_constants.py file in favor of amundsen-common's index_map.py. If you were using databuilder's constants, please switch to common.

Dmitriy Kunitskiy added 2 commits June 16, 2021 12:35
Signed-off-by: Dmitriy Kunitskiy <dkunitskiy@lyft.com>
Signed-off-by: Dmitriy Kunitskiy <dkunitskiy@lyft.com>
Copy link
Contributor

@allisonsuarez allisonsuarez left a comment

Choose a reason for hiding this comment

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

LGTM

@allisonsuarez allisonsuarez merged commit 79792cd into amundsen-io:main Jun 16, 2021
amommendes pushed a commit to amommendes/amundsen that referenced this pull request Jul 22, 2021
…1241)

* fix: update user elasticsearch index

Signed-off-by: Dmitriy Kunitskiy <dkunitskiy@lyft.com>

* bump version

Signed-off-by: Dmitriy Kunitskiy <dkunitskiy@lyft.com>
Signed-off-by: Amom <amommendes@hotmail.com>
zacr pushed a commit to SaltIO/amundsen that referenced this pull request May 13, 2022
…1241)

* fix: update user elasticsearch index

Signed-off-by: Dmitriy Kunitskiy <dkunitskiy@lyft.com>

* bump version

Signed-off-by: Dmitriy Kunitskiy <dkunitskiy@lyft.com>
hansadriaans pushed a commit to DataChefHQ/amundsen that referenced this pull request Jun 30, 2022
…1241)

* fix: update user elasticsearch index

Signed-off-by: Dmitriy Kunitskiy <dkunitskiy@lyft.com>

* bump version

Signed-off-by: Dmitriy Kunitskiy <dkunitskiy@lyft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:common From common folder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants