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

Devshell #759

Merged
merged 10 commits into from May 15, 2020
Merged

Devshell #759

merged 10 commits into from May 15, 2020

Conversation

@gilligan
Copy link
Contributor

gilligan commented May 14, 2020

overview

This PR adds a foreman based setup to easily start hydra from the working copy.

motivation

The goal of this PR is to make it dead simple to run Hydra from the working copy. This makes for a much better development and testing experience. It doesn't require any additional permissions, virtualization or networking.

  • foreman start: this can be executed in the nix-shell when you don't want to make us of incremental builds via make. It uses all hydra binaries from the working copy.

details

  • hydra is stared on port 63333 to avoid collisions
  • postgres is started on port 64444 to avoid collisions
  • the nix-shell environment now has a foreman which can be used to start all required hydra services and postgresql
  • foreman is configured in Procfile
@gilligan gilligan force-pushed the gilligan:devshell branch from c3ef09b to e03632d May 14, 2020
@edolstra
Copy link
Member

edolstra commented May 14, 2020

Not in favor of this. It brings in another (non-systemd) service management framework which seems brittle and painful to maintain.

@grahamc
Copy link
Member

grahamc commented May 14, 2020

I dunno, is there an easy way to run the various hydra components + postgres under systemd for a dev-only environment, without reconfiguring the whole system? I don't know of one. I'm pretty excited about this PR -- being able to enter the shell and run foreman is a really nice way to develop the software.

@gilligan
Copy link
Contributor Author

gilligan commented May 14, 2020

@edolstra I'll agree that it isn't perfect that this adds a dependency on an (already packaged) Ruby program. However:

  • the "framework" is really primitive/small surface (see Procfile)
  • i would argue it provides a drastic improvement of developer experience
  • there have been other attempts at providing something similar which were either less convenient/and or much more involved or required additional setup/permissions: #574 and #353. I would consider this PR a better approach.

I generally go by "perfection is the enemy of good". This isn't perfect but I think it is as good as it gets in terms of providing a really easy and convenient way of working with hydra. This also seems like a very relevant factor in attracting contributors and encouragement to work on Hydra.

I am more than happy to demo it you if you want. it's quite nice, i promise ;)

@gilligan
Copy link
Contributor Author

gilligan commented May 14, 2020

FWIW: I am happy to remove runHydra and limit this to calling foreman in the normal shell environment. What i want to emphasize here is:

This isn't about introducing some novel way of running Hydra. This is strictly about improving the development workflow experience by providing an easy way to run hydra from your workspace without requiring any additional setup.

I explored different ways (as did others, see the PRs i mentioned above) but this seems like the best solution to me.

scripts/start-hydra.sh Outdated Show resolved Hide resolved
scripts/start-hydra.sh Outdated Show resolved Hide resolved
scripts/start-queue-runner.sh Outdated Show resolved Hide resolved
@grahamc
Copy link
Member

grahamc commented May 14, 2020

I've checked this PR out and it looks pretty good. I find Hydra pretty annoying to run, and "okay time to look at a bug in Hydra" always starts with a sigh and "okay ... where did I put my notes about how to dev Hydra?"

If we create a default.nix in tests/, then this jobset configuration works and creates a build:

image

@samueldr
Copy link
Member

samueldr commented May 14, 2020

I'm curious if there was inspiration from https://github.com/samueldr/hydra-in-a-bag or if it was only concurrent evolution of the same idea. In any case, it's great to see this in the upstream project, rather than a side-project.

EDIT: and psst, yes, if there is anything useful, do feel inspired and take it!

@gilligan
Copy link
Contributor Author

gilligan commented May 14, 2020

@samueldr hah, cool. Now that you mention it, i do remember this project! I was chatting with @andir and we explored some options that didn't lead anywhere. However graham had mentioned in passing that he once tried to use foreman for this. That's how we ended up with that.

I do think i once tried your hydra-in-a-bag solution but i had totally forgotten about it really ;) Thanks for reminding me!

@gilligan
Copy link
Contributor Author

gilligan commented May 14, 2020

FWIW I wouldn't have a problem nixifying our solution in the same way that @samueldr chose. I think we have some improvements in the way how we wait for the database and that we don't have a separate DB initialization step. But all that could be adjusted.

could be done. could also be done in a follow-up PR. But in any case only once if we can reach an agreement that this is desirable.

@samueldr
Copy link
Member

samueldr commented May 14, 2020

Now that I have had some time to read through the discussion more in details, here's my thoughts.

This is not a way to run a production or production-like Hydra instance. This is a way to compartmentalize a development instance of Hydra in a trivial manner, where its state is all self-contained.

(In this approach the state is hidden in an hidden directory, which I think is probably a minor issue. Showing the state directory front and center would probably be better in exposing it to the user.)

Now, I believe that this kind of self-contained and self-starting development environment is the basic courtesy we should give to anyone wanting to contribute to the Hydra project. Anything more complex to setup, requiring either a different host (server, vm) or a reconfiguration of the host system is a non-starter. It is too much friction to just fix a thing. Even more, this is a good way to get a test instance running to quickly just try something in Hydra.

Assuming that this ends up being undesirable inside the Hydra project, I guess an -in-a-bag kind of setup at nix-community could do, but this reduces its discoverability.

shell.nix Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
Procfile Outdated Show resolved Hide resolved
@edolstra
Copy link
Member

edolstra commented May 15, 2020

I am happy to remove runHydra and limit this to calling foreman in the normal shell environment.

Yes please. For the use case of incremental development runHydra isn't necessary (or even a good idea) and it might tempt people to start using it in production.

@gilligan gilligan mentioned this pull request May 15, 2020
@Mic92
Copy link
Contributor

Mic92 commented May 15, 2020

Not in favor of this. It brings in another (non-systemd) service management framework which seems brittle and painful to maintain.

In theory it would be possible to use systemd --user or systemd-run however they don't work on other operating system, which makes porting painful.

andir and others added 8 commits May 13, 2020
runHyda automatically starts hydra and postgres:

```
$ nix-shell -A runHydra
```

The shell receives hydra from the working copy as buildInput.
Running hydra, queue-runner, evaluator and postgres is managed
by foreman (https://github.com/ddollar/foreman) and configured
in `Procfile`.
This adds a `devShell` which unlike `runHydra` doesn't start hydra
automatically and doesn't receive hydra as build input. It is better
suited for interactive development cycles:

```
$ nix-shell -A devShell
$ ./bootstrap
$ configurePhase
$ make
$ # hack hack hack
$ foreman start
  # test test test
  <C-c>
$ # hack hack hack
```
Use custom ports so hydra and postgres can run in environments where
the default ports are in use already.
Add sections about using `runHydra` and `foreman`
Using `pg_ctl status` is more reliable than relying checking an
open port via netcat.
Co-authored-by: Graham Christensen <graham@grahamc.com>
Execute hydra-dev-server instead of hydra-server

Co-authored-by: Graham Christensen <graham@grahamc.com>
flake.nix Outdated Show resolved Hide resolved
- scripts -> foreman
- drop runHydra
- drop devShell
- move postgresql to buildInputs
@gilligan gilligan force-pushed the gilligan:devshell branch from 6519af0 to 31262f1 May 15, 2020
- drop any mention of runHydra
- link foreman and mention Procfile
@gilligan
Copy link
Contributor Author

gilligan commented May 15, 2020

@edolstra alright..

  • runHydra is gone
  • devShell is gone
  • shell.nix refers to the shellNix attribute from flake-compat
  • the scripts are now in ./foreman
  • i updated the README

I could imagine to encapsulate everything with Nix in the same way that @samueldr does it with hydra-in-a-bag but I would prefer to try that separately and if it turns out well offer a follow-up PR for you/everyone to look at.

@edolstra edolstra merged commit c4104fe into NixOS:master May 15, 2020
2 checks passed
2 checks passed
tests
Details
validate-openapi
Details
@edolstra
Copy link
Member

edolstra commented May 15, 2020

Thanks @gilligan!

@gilligan gilligan deleted the gilligan:devshell branch May 15, 2020
@knl
Copy link
Contributor

knl commented May 15, 2020

Not in favor of this. It brings in another (non-systemd) service management framework which seems brittle and painful to maintain.

In theory it would be possible to use systemd --user or systemd-run however they don't work on other operating system, which makes porting painful.

Maybe I missed that announcement, but does Hydra work on anything that is not Linux + systemd? If yes, could you please add some docs on how to do it?

@edolstra
Copy link
Member

edolstra commented May 15, 2020

It might work but it's not supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.