rainforestapp / fourchette

DEPRECATED - Your new best friend for isolated testing environments on Heroku.
MIT License
194 stars 23 forks source link

Callbacks don't appear to run #45

Closed seanknox closed 9 years ago

seanknox commented 9 years ago

First off: excited to use this in our org—turns out we solved this same problem for our team at almost the same time you did, although yours is better in many ways! :) The @CircleCI folks tipped me off about fourchette.

As alluded to in https://github.com/rainforestapp/fourchette/issues/44, I need to run an after callback to copy the Postgres DB from the source Heroku app to the newly created app. Forking the DB is not an option. Looks like fourchette already does this but it fails (#42)?

In any case, in my app generated with fourchette my-app-name, the callbacks don't appear to be running. Here's my callbacks file:

class FourchetteCallbacks
  include Fourchette::Logger

  def initialize(params)
    @params = params
    @heroku = Fourchette::Heroku.new
  end

  def before_all
    logger.info 'Placeholder for before steps... (see callbacks.rb to override)'
  end

  def after_all
    logger.info "Copying database from #{source_app_name} to #{new_app_name}..."
    pg_backup.copy(source_app_name, new_app_name)
  end

  private

  def heroku_fork_obj
    @heroku_fork ||= Fourchette::Fork.new(@params)
  end

  def new_app_name
    @fork_name ||= heroku_fork.fork_name
  end

  def source_app_name
    ENV.fetch('FOURCHETTE_HEROKU_APP_TO_FORK')
  end

  def pg_backup
    Fourchette::Pgbackups.new
  end
end

I pushed my code to Heroku and opened a PR to cue creation of an app, but the logs show:

2014-11-14T02:34:05.657332+00:00 app[web.1]: I, [2014-11-14T02:34:05.657210 #23]  INFO -- : Cloning repository...
2014-11-14T02:34:09.216835+00:00 app[web.1]: I, [2014-11-14T02:34:09.216679 #23]  INFO -- : Preparing tarball...
2014-11-14T02:34:10.148992+00:00 app[web.1]: tar: .: file changed as we read it
2014-11-14T02:34:10.153502+00:00 app[web.1]: I, [2014-11-14T02:34:10.153329 #23]  INFO -- : Tarball to URL as a service in progress...
2014-11-14T02:34:11.032727+00:00 app[web.1]: I, [2014-11-14T02:34:11.032622 #23]  INFO -- : Start of the build process on Heroku...
2014-11-14T02:34:11.069947+00:00 app[web.1]: I, [2014-11-14T02:34:11.069797 #7]  INFO -- : Serving a tarball!
2014-11-14T02:34:41.254423+00:00 app[web.1]: I, [2014-11-14T02:34:41.254280 #23]  INFO -- : Placeholder for after steps...
...

Looks like the default after callback in the gem is running. Assuming the callbacks defined in my app should override the gem's, but that doesn't appear to be the case.

jipiboily commented 9 years ago

Are you sure it is being loaded? Is your Fourchette app code available somewhere or it's closed source?

jipiboily commented 9 years ago

Also, glad you're excited about it! Who @ CircleCI told you about Fourchette? They're great people and we use & love them too :D

seanknox commented 9 years ago

The app was deployed—although I when opened the issue didn't realize that it can take awhile to compile the slug (I'm used to forking as opposed to building).

re: CircleCI, it was Jonathan Irving!

jipiboily commented 9 years ago

@seanknox

Yes, it's accurate that we (Rainforest QA) have been running into issues with Heroku's DB transfer API that is used in Fourchette. I didn't have time to fix it properly yet and I don't know when I'll have to.

Callbacks are called before and after any action, you can see it here: https://github.com/rainforestapp/fourchette/blob/78ab379ec20ce226bbedea95d84f633665a1c461/lib/fourchette/pull_request.rb

That said, it might make a lot of sense to have different callbacks for different type of pull request actions.

Hope that helps?

seanknox commented 9 years ago

Right, so it looks like callbacks have to be defined in the gem itself? The callbacks defined in the Sinatra app are never triggered.

My fourchette gem callbacks (the default):

class Fourchette::Callbacks
  include Fourchette::Logger

  def initialize params
    @params = params
  end

  def before_all
    logger.info 'Placeholder for before steps...'
  end

  def after_all
    logger.info 'Placeholder for after steps...'
  end
end

My callbacks in the Fourchette Sinatra app:

class FourchetteCallbacks
  include Fourchette::Logger

  def initialize(params)
    @params = params
  end

  def before_all
    logger.info 'Placeholder for before steps... (see callbacks.rb to override)'
  end

  def after_all
    logger.info "Copying database from #{source_app_name} to #{new_app_name}..."
    pg_backup.copy(source_app_name, new_app_name)
    remove_airbrake_api_var
  end

  private

  def heroku
    @heroku ||= Fourchette::Heroku.new
  end

  def remove_airbrake_api_var
    # Don't enable Airbrake reporting for QA apps.
    heroku.client.remove_config_var(new_app_name, 'AIRBRAKE_API_KEY')
  end

  def heroku_fork_obj
    @heroku_fork ||= Fourchette::Fork.new(@params)
  end

  def new_app_name
    @fork_name ||= heroku_fork.fork_name
  end

  def source_app_name
    ENV.fetch('FOURCHETTE_HEROKU_APP_TO_FORK')
  end

  def pg_backup
    Fourchette::Pgbackups.new
  end
end

The Sinatra callbacks are never run—the logs always show 'Placeholder for after steps...' instead of the output from my custom callbacks.

jipiboily commented 9 years ago

Are you requiring them? They are not going to be autoloaded. It definitely works with the latest stable Fourchette version as we're using it right now.

Here is how it's setup for us:

jipiboily commented 9 years ago

Ah, re-reading, you are not overriding the right class. You are creating FourchetteCallbacks while you should do it on Fourchette::Callbacks :grinning:

seanknox commented 9 years ago

@jipiboily Yep just found that myself! Thanks for the extra eyes.

jipiboily commented 9 years ago

Glad you've got it sorted out :)