opened
hogum
username_0
#### Fixes #1889

This cleans up the uses of the field `email` from `User` by:
- dropping the `email` field from `User`
- clearing references to the dropped email field
- migrating and updating the schema changes
Drop Email column
issue
created
rust-highfive
username_1
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @username_2 (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see [the contribution instructions](https://github.com/rust-lang/crates.io/blob/master/docs/CONTRIBUTING.md) for more information.
comment
created
hogum
username_0
How should the `database visibility configuration` be updated after modifying the schema?
Its test fails with:
```
---- tasks::dump_db::gen_scripts::tests::check_visibility_config stdout ----
thread 'tasks::dump_db::gen_scripts::tests::check_visibility_config' panicked at 'The visibility configuration does not match the database schema:
Column users.email does not exist in the database.', src/tasks/dump_db/gen_scripts.rs:204:9
```
comment
created
carols10cents
username_2
Removing [the users.email line from the dump-db.toml file](https://github.com/rust-lang/crates.io/blob/23f187ac703cd2e4e36621729f550f4c6f8be9de/src/tasks/dump_db/dump-db.toml#L168) should do it. The dump-db.toml file is configuration for the public database dumps we make, and this test ensures the database schema is always in sync with this configuration so that we don't accidentally leak data. 

I'll make the error message for that test a bit clearer to point at what needs to be done.
comment
created
hogum
username_0
Thanks @username_2! That sorted it out.
Dumping to the emails table originally required the `user: email` column - what would be the reason for getting a unique key violation on `emails: user_id` after dropping the `users: email` column. 

https://github.com/rust-lang/crates.io/blob/23f187ac703cd2e4e36621729f550f4c6f8be9de/migrations/20170804200817_add_email_table/up.sql#L16-L17
comment
created
carols10cents
username_2
I'm confused, are you getting a unique key violation? Can you give me steps to reproduce?
comment
created
carols10cents
username_2
Oh I see the errors in travis now. Investigating!
comment
created
hogum
username_0
Resolved. The invalid email address was a mistake I hadn't noted. Is it not supposed to be caught? I'm surprised it passes the test
comment
created
hogum
username_0
Resolved. The invalid email address was a mistake I hadn't noted. Is it not supposed to be caught? I'm surprised it passes the test
comment
created
carols10cents
username_2
Thank you so much!! This looks great, but I'm not going to merge it yet-- we need to merge and deploy https://github.com/rust-lang/crates.io/issues/1888 to remove all the *uses* of the email column first, so that there aren't any connections using the `email` field when we deploy this PR.
comment
created
bors
username_3
:umbrella: The latest upstream changes (presumably #1901) made this pull request unmergeable. Please resolve the merge conflicts.
comment
created
bors
username_3
:umbrella: The latest upstream changes (presumably #1700) made this pull request unmergeable. Please resolve the merge conflicts.
comment
created
carols10cents
username_2
I removed some more of the temporary code that was needed to deploy the code that stopped using the email column (and that has been deployed). I also introduced more merge conflicts by merging another PR just now; I'll be fixing those in a few hours.
comment
created
carols10cents
username_2
I think this is ready to go now!! Thank you!!!

@username_3 r+
comment
created
bors
username_3
:pushpin: Commit c7ac5cb1411191f243c80b27488fc6e76469db05 has been approved by `username_2`

<!-- @username_3 r=username_2 c7ac5cb1411191f243c80b27488fc6e76469db05 -->
<!-- homu: {"type":"Approved","sha":"c7ac5cb1411191f243c80b27488fc6e76469db05","approver":"username_2"} -->
comment
created
bors
username_3
:hourglass: Testing commit c7ac5cb1411191f243c80b27488fc6e76469db05 with merge dbe39ae3e5c58c2c963a062e59a32ec6ac62200f...
<!-- homu: {"type":"BuildStarted","head_sha":"c7ac5cb1411191f243c80b27488fc6e76469db05","merge_sha":"dbe39ae3e5c58c2c963a062e59a32ec6ac62200f"} -->
comment
created
bors
username_3
:sunny: Test successful - [checks-travis](https://travis-ci.com/rust-lang/crates.io/builds/139021512)
Approved by: username_2
Pushing dbe39ae3e5c58c2c963a062e59a32ec6ac62200f to master...
<!-- homu: {"type":"BuildCompleted","approved_by":"username_2","base_ref":"master","builders":{"checks-travis":"https://travis-ci.com/rust-lang/crates.io/builds/139021512"},"merge_sha":"dbe39ae3e5c58c2c963a062e59a32ec6ac62200f"} -->
comment