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

When running database commands in development env, apply them to the test database too #215

Closed
timriley opened this issue Jul 20, 2024 · 10 comments · Fixed by #247
Closed
Assignees

Comments

@timriley
Copy link
Member

Currently the user user needs to run pairs of commands every time they tend to their local database:

hanami db create
HANAMI_ENV=test db create

hanami db migrate
HANAMI_ENV=test db migrate

# etc...

This is unhelpful friction, and not a great first impression on any new users.

Instead, when db commands are run and the HANAMI_ENV is development, then we should also apply those commands to the test database.

This should take effect for the following commands:

  • db create
  • db drop
  • db migrate
  • db prepare
  • db seed
  • structure load

It should not take effect, however, for these commands, since these only really make sense to do for a single database:

  • structure dump
  • version
@solnic solnic self-assigned this Jul 22, 2024
@solnic
Copy link
Member

solnic commented Jul 23, 2024

@timriley do we want to solve this in hanami-cli by tweaking DB::Command#run_command or do we want to add a first class feature to dry-cli for re-running a command with a different env and just use that in Hanami?

@timriley
Copy link
Member Author

timriley commented Jul 24, 2024

@solnic Thanks for asking! I think we should make it specific to our hanami db commands only.

I think this is a requirement that's specific to a subset of the db commands only, and not really something that makes sense to generalise to dry-cli.

In addition, I think we want the handling of errors/output/return values, etc. to be internal to the command class as it iterates over the databases, which is something that I think would be more difficult to achieve in a nice way if we pushed this behaviour down a layer into dry-cli.

@solnic
Copy link
Member

solnic commented Jul 24, 2024

@timriley is it possible to reconfigure an app with a different env in the same process? I've tried that and it seems too hard at the moment. I'm not even sure if it's feasible to do. Any tips how this could be done? There's no easy way of applying a command with different options. I naively thought it would be a matter of doing command.call(*args, **opts.merge(env: :test)) but it's not gonna work since a command sets up the app and memoize it, so when you try to rerun a command, it's gonna use the app in development mode. I managed to re-setup the app in test env but slices containers remained in development mode, so something still has development env set.

@francois
Copy link
Contributor

One solution would be to fork or exec from within the development process and pass a different HANAMI_ENV.

The biggest pain point at the moment is running specs when the test DB is different from the development one. Rails checks that the test DB matches development and fails with a "the test DB needs to migrate; run bin/rails db:migrate RAILS_ENV=test". This would be a first line of defense against surprising behaviour.

An alternative is to reload the database structure in the test database when starting the specs, but that gets slow fast when the database hasn't changed.

@timriley
Copy link
Member Author

@solnic Thanks for the extra thoughts here. You're right: this is tricky!

The idea from @francois would be one solution, but I'd hope we wouldn't have to go this far, especially since I think it would give us a bunch of extra work around handling command output and possible failure handling.

I'll have a muse on what else might be possible here and share some more thoughts in a few days :)

@timriley
Copy link
Member Author

OK, I think something similar to @francois’ approach is the way to go here: at the end of each relevant db command's #call class, if it has been run in development mode, then invoke a new system call to the same CLI command (i.e. running a whole new shell command) with HANAMI_ENV=test set for that execution.

Trying to do this all within the same process won't be straightforward right now. To do this, we would need to do one of these things:

  1. Shift all of our database setup and connection logic behind dedicated APIs within Hanami::DB, so that we can invoke those directly from our CLI Commands, and pass the appropriate Hanami env to each one.
  2. Or make it possible to boot up copies or parallel instances of Hanami apps with their own distinct environments.

I would actually like to do (1) sooner rather than later, but I don't think we should block 2.2 on this. It would be a nice enhancement to add at any time, but it's not strictly necessary to add right now.

As for (2), that would also be nice to experiment with, but is possibly even more work than (1) and it's definitely not important for anything else in this release, so let's save that for the future.

So given all of this, if we can achieve the goal with the simple measure of executing a second full shell command at the end of the first one, this gets us this (very desirable) user feature with the least effort for now.

@solnic would you be up for giving that a go?

@timriley
Copy link
Member Author

timriley commented Sep 7, 2024

@solnic Are you still interested in working on this, or should we pull it back into the to-do list for others to do?

@swilgosz
Copy link
Member

@timriley - are you sure this should take effect on db:seed ? Hanami 2.2 will have database cleaner configured, which will clean up db content anyway, adding just unnecessary slow down for seeding.

Second question: What about adding a flag -t or --test, that will only then run it for TEST env?

@swilgosz
Copy link
Member

Adding this at the end of command this snippet

# lib/hanami/cli/commands/app/db/create.rb

  return unless ENV.fetch("HANAMI_ENV", "development")
  cmd, *args = test_command(app, slice, gateway)
  system_call.call(cmd, *args,
    env: { 'HANAMI_ENV' => 'test'}
  )

works in terms it creates the _test database, but the execution hangs, never returning the output until interrupted.

@timriley timriley self-assigned this Oct 24, 2024
@timriley
Copy link
Member Author

@swilgosz I think we might need an interactive_system_call here, or perhaps just some call to a lower-level Ruby API. I'll have a go now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants