Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Stop writing to column user_id of tables profiles and user_filters#16181

Open
H-Shay wants to merge 6 commits into
developfrom
shay/stop_writing_user_id
Open

Stop writing to column user_id of tables profiles and user_filters#16181
H-Shay wants to merge 6 commits into
developfrom
shay/stop_writing_user_id

Conversation

@H-Shay
Copy link
Copy Markdown
Contributor

@H-Shay H-Shay commented Aug 24, 2023

This is in preparation to drop the user_id column from these tables entirely. This was originally introduced in #15787 but was reverted in #15953 because I found some places where we were still reading from the column user_id of the table profiles. These reads were then dropped in #15955 which was part of v1.89.0 so we should be able to stop writing to these columns now, and drop the column entirely next release cycle.

@H-Shay H-Shay requested a review from a team as a code owner August 24, 2023 22:25
self.get_success(self.store.get_profile_avatar_url(self.u_frank))
)

def test_profiles_bg_migration(self) -> None:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The rationale for dropping these tests is that they both were testing background updates that have already run, and the changes to the code in this PR result in making it pretty impossible to run the tests in sqlite. When I added these tests I felt like they were pretty borderline useful (basically only as a proof of concept) so I think it makes sense to drop them now.

@reivilibre reivilibre self-assigned this Aug 31, 2023
@clokep clokep removed the request for review from a team September 6, 2023 19:20
@clokep
Copy link
Copy Markdown
Member

clokep commented Sep 6, 2023

Removed this from the review queue since @reivilibre is taking it over and it has conflicts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants