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

Add isFile column in url table #53

Merged
merged 1 commit into from
May 4, 2020
Merged

Conversation

yong-jie
Copy link
Member

@yong-jie yong-jie commented Apr 27, 2020

Problem

We need to introduce a column in the url table to indicate whether a shortlink is pointing to a file.go.gov.sg object.

Solution

Create two scripts. The first would be to add the isFile column in a backwards-compatible way. This is done by making the column nullable(already true by default). After future PRs that update the codebase to start writing into the isFile column, run the second script to backfill and set the not-null constraint.

Follow up

I'll PR the next step which updates the url model after the migration strategy has been finalised and merged.

@yong-jie yong-jie requested a review from liangyuanruo April 27, 2020 18:46
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

Thought through the use of DEFAULT false, still have to

  • backfill false values for standard short links
  • backfill url_history

@yong-jie
Copy link
Member Author

@liangyuanruo I've changed the approach back to the one you were suggesting, involving two steps. The first being to set a nullable column in both tables, and then the second script to backfill + add not-null constraint.

@yong-jie yong-jie requested a review from liangyuanruo April 28, 2020 07:26
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

As commented

@yong-jie yong-jie force-pushed the isfile-column-migration branch 2 times, most recently from 647ee24 to 86b4565 Compare May 3, 2020 14:37
@yong-jie yong-jie changed the base branch from file-upload-master to develop May 3, 2020 14:38
@yong-jie yong-jie force-pushed the isfile-column-migration branch from 86b4565 to b93c22e Compare May 3, 2020 14:45
@yong-jie yong-jie requested a review from liangyuanruo May 3, 2020 18:28
@yong-jie
Copy link
Member Author

yong-jie commented May 3, 2020

target branch has been changed to develop, requesting review before merge so i can get the next steps up

Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

lgtm!

@liangyuanruo liangyuanruo merged commit c251d5e into develop May 4, 2020
@liangyuanruo liangyuanruo deleted the isfile-column-migration branch May 4, 2020 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants