-
Notifications
You must be signed in to change notification settings - Fork 23
Updating fourchette to work with migrated Heroku pg:backups #57
Conversation
@@ -30,6 +30,12 @@ def client | |||
@heroku_client ||= PlatformAPI.connect(api_key) | |||
end | |||
|
|||
def legacy_client | |||
api_key = ENV['FOURCHETTE_HEROKU_API_KEY'] | |||
ENV['HEROKU_API_KEY'] = api_key # necessary for Heroku::Auth.password to work |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is that necessary? @non_platform_client
is not even used anywhere in that PR...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, nevermind, it's used... :P
but why is the ENV['HEROKU_API_KEY']
required is still a valid question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find support in their platform API for the migrated PG backups, so I was forced to use the ruby client. One of the problems with it is that its Heroku::Client::HerokuPostgresql implicitly uses Heroku::Auth, which tries to read credentials from stdin. Only means I could see of bypassing this behavior is setting this environment variable before it does so.
Thanks for the PR! The build is failing. I added a couple of questions. I will need more time to test it. At first, it seems fine. @scruti @thinkmorebetter: could you look at it and maybe try it on your apps? Would be awesome! Thanks |
No problem. It's still a work in progress, I was planning on fixing the tests, and using it a bit more with our app to iron out any issues (or example, our app needs 2X dynos, so I was hoping on getting that working). |
2 similar comments
@jipiboily I've fixed the specs - what do you think about merging this? |
1 similar comment
@wless1 Gave this branch a shot and ended up with the following error
Not sure if I am doing something wrong, but the stack trace seems to suggest that it is a result |
@client.create_transfer(from_url, from_name, to_url, to_name) | ||
end | ||
raw_attachment = @heroku.legacy_client.get_attachments(to).body[0] | ||
attachment = Heroku::Helpers::HerokuPostgresql::Attachment.new raw_attachment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wless1 Getting the 0 indexed attachment does not work for me. We should probably analyze each attachment until we find the attachment with a name starting with HEROKU_POSTGRESQL
@wless1 sorry it's been so long. What's the status of this? Is this working for you? |
FYI, we just decided to deprecate Fourchette. You can (and probably should) use the Heroku review apps instead which was inspired by Fourchette. https://devcenter.heroku.com/articles/github-integration-review-apps Thanks! |
Closing, feel free to re-open if you think it's worth pursuing. |
No description provided.