Page MenuHomePhabricator

Add command to start MySQL on linux
ClosedPublic

Authored by jon on May 4 2022, 12:55 AM.
Tags
None
Referenced Files
F3380643: D3902.diff
Thu, Nov 28, 1:07 AM
Unknown Object (File)
Tue, Nov 26, 9:35 AM
Unknown Object (File)
Sun, Nov 24, 10:30 AM
Unknown Object (File)
Sun, Nov 24, 10:23 AM
Unknown Object (File)
Sun, Nov 24, 9:37 AM
Unknown Object (File)
Sun, Nov 24, 9:37 AM
Unknown Object (File)
Sun, Nov 24, 9:36 AM
Unknown Object (File)
Sun, Nov 24, 9:22 AM

Details

Summary

Allow for the creation of a local MySQL instance by just running a unprivileged command.

Will do something similar for mac+launchd soon.

Depends on D3874

Test Plan

nix run .#mysql-up to bring up the server
journalctl -f --user-unit comm-mysql to view mysql logs
nix run .#mysql-down to bring the server down

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.May 4 2022, 12:58 AM
Harbormaster failed remote builds in B8747: Diff 12188!

Be more consistent about runtime unix socket path

Harbormaster returned this revision to the author for changes because remote builds failed.May 4 2022, 1:04 AM
Harbormaster failed remote builds in B8748: Diff 12189!

Rebase work to exclude grpc WIP commits

ashoat requested changes to this revision.May 4 2022, 11:15 AM

Interesting... I'm surprised these utilities don't exist upstream anywhere, since it feels like anybody with a dev environment that relies on MySQL will need them

nix/mysql-down-linux.nix
5–7

Thanks for this comment, it's really helpful!

nix/mysql-up-linux.nix
20

Are you intentionally using the port 3006 rather than the default 3306? (Assuming yes, but figured I'd check)

34

I assume we're using bash-specific syntax below... does this need to be made explicit anyways?

42

I think it's preferable to use [[ ]] over [ ]

This revision now requires changes to proceed.May 4 2022, 11:15 AM

Interesting... I'm surprised these utilities don't exist upstream anywhere, since it feels like anybody with a dev environment that relies on MySQL will need them

https://search.nixos.org/options?channel=21.11&from=0&size=50&sort=relevance&type=packages&query=mysql

They exist at the NixOS (e.g. machine/system) level, but they don't within Nixpkgs.

nix/mysql-up-linux.nix
20

This I probably fat fingered, to be honest. I tested it using mysql but that uses the UNIX socket by default.

jon marked 4 inline comments as done.

Use 3306 default mysql port

Additional bash cleanup

nix/mysql-down-linux.nix
5–7

:)

nix/mysql-up-linux.nix
34

In practice, it will be bash, but I don't think I use any bashisms

42

I believe [[ ]] could be less portable, but looks like zsh and ksh also support it

Add comm database and user creation logic

ashoat requested changes to this revision.May 6 2022, 11:45 AM
ashoat added inline comments.
nix/mysql-up-linux.nix
20 ↗(On Diff #12336)

Is --port 3306 even necessary since it's the default? (I don't feel strongly about this... happy to keep it here if you want to be explicit)

42 ↗(On Diff #12336)

Any reason we're not using [[ ]] here? (When somebody asks for changes on one line, you should always search for other cases of the same thing and apply the feedback everywhere)

67 ↗(On Diff #12336)

I was confused about this use of the double-single-quotes. I did some research and looks like it's a way of escaping the $ character. In case it's interesting to other reviewers, more details here

73 ↗(On Diff #12336)

Can we also run cd keyserver && yarn dist/scripts/create-db.js here to set up the tables?

(To do this we would need to save the DB credentials to keyserver/secrets/db_config.json, and would otherwise need the repo to be initialized with yarn cleaninstall && cd keyserver && yarn prod-build)

This revision now requires changes to proceed.May 6 2022, 11:45 AM
jon added inline comments.
nix/mysql-up-linux.nix
42 ↗(On Diff #12336)

looks like I missed this one, got the other 3

67 ↗(On Diff #12336)

Correct, if I want to fully escape a $ in a "lines" block, then I need to use ''$ to do so.

Although, in this case, it's kind of a noop, as nix only changes the behavior when you use the ${...} interpolation syntax.

73 ↗(On Diff #12336)

right now I'm just using the insecure initialization since it's a dev environment, which means user: root; pass: "". But I can run anything if needed, just transcribed what was in the dev env doc.

Address looked-over bracket

Before landing, can you create a Linear task for the MySQL initialization?

Some tips for the task creation:

  1. See "Guidelines for new Linear issues" here
  2. Probably should be a child of ENG-177
  3. Should be within the "Nix environment" project

Some stuff you might want to put in there:

  1. Secure initialization (what you talked about)
  2. Creation of MySQL user / database
  3. Population of keyserver/secrets/db_config.json config file
  4. Creation of tables by running yarn cleaninstall && cd keyserver && yarn prod-build && yarn script dist/scripts/create-db.js
This revision is now accepted and ready to land.May 11 2022, 6:14 PM
This revision was automatically updated to reflect the committed changes.

This should not have been landed without addressing my comment!! I left an explicit comment saying "Before landing, can you create a Linear task for the MySQL initialization?"

This calls into question if we can trust @jonringer-comm with an "accept with comments". @jonringer-comm, please keep this in mind going forward. Just because a diff is accepted, does NOT mean you can just land it. You ALWAYS need to check the comments and address any final requests BEFORE landing.

@varun, given @jonringer-comm is out for his honeymoon, can you handle the task creation detailed above?

Realizing now that ENG-1134/ENG-1135 may have been intended as those tasks, but I didn't realize it since they don't have all the details requested above and weren't linked from here. Please always link requested tasks before landing!