phallstrom / slackistrano

Slack integration for Capistrano deployments.
MIT License
374 stars 74 forks source link

"You are using an outdated configuration that will be removed soon" #95

Open jrochkind opened 5 years ago

jrochkind commented 5 years ago

I currently get this in my cap output:

      WARN  [slackistrano] You are using an outdated configuration that will be removed soon.
      WARN  [slackistrano] Please upgrade soon! <https://github.com/phallstrom/slackistrano>

Visiting the home page at https://github.com/phallstrom/slackistrano, I sort of expected to see a big announcement at the top or something. But it's unclear to me how to figure out what about my configuration is outdated or how to upgrade. Any advice?

Or is this just telling me to upgrade the slackistrano gem? I have 3.8.3, ah I see there is a 4.0, is it just asking me to upgrade to that?

phallstrom commented 5 years ago

@jrochkind Gem version should be fine. It's your configuration itself. You're using the old multiple keys stuff instead of the new single slackistrano key that takes a hash. See the README for new examples.

jrochkind commented 5 years ago

I am having trouble figuring out what you're talking about.

My slackistrano configuration looks like this:

        set :slackistrano, {
          klass: ScihistDigicoll::SlackistranoMessaging,
          webhook: ENV["SLACK_NOTIFICATION_WEBHOOK"]
        }

That looks like one key that takes a hash? Or are we talking about something else? I am using a custom messaging class, as you can see. Could it be related something there?

The deprecation message is very unclear, having trouble figuring out what I am doing that is deprecated. I suppose I could debug the source code interactively to see at what point slackistrano is deciding to issue that deprecation message, and why it's deciding to.

The only difference I can see between README examples and me is that I am not providing a :channel key. This is because my custom messaging class determines the channel (deciding different channels for different environments).

jrochkind commented 5 years ago

Hm, it's possible I'm only getting the deprecation message on a deploy failure notification, could that be?

phallstrom commented 5 years ago

Hrm. Let me dig in. Can you share your custom class?

jrochkind commented 5 years ago

It's all open source, although my code gets a little bit weird. Am trying to debug myself.

Might have to reproduce a capistrano failure to see it, it may be I be only getting the message on failure (which might give you a clue to debug too). I did just have a cap failure due to an infrastructure problem, which was when I noticed the message I had never noticed before. My guess is it's related more to that than to my custom code, but:

https://github.com/sciencehistory/scihist_digicoll/blob/master/lib/scihist_digicoll/slackistrano_messaging.rb

https://github.com/sciencehistory/scihist_digicoll/blob/master/config/deploy.rb#L95-L112

(I forget why I put the slackistrano registration in a cap task -- I think I didn't have access to some configuration or metadata I needed without doing that -- the revisions for the githut diff link maybe? Including link to github diff is neat, right? But maybe that's what's causing the erroneous deprecation -- only on failure? Except I succesfully get the failure notification in slack!)

phallstrom commented 5 years ago

Strange. You're setting it to false which should use the Null messaging class, but the only way to get that warning is if the config is empty. Which makes me think either I have a bug in 3.8.3 that turns false into nil or capistrano has changed to do similar.

See: https://github.com/phallstrom/slackistrano/blob/v3.8.3/lib/slackistrano/capistrano.rb#L24

I wonder if you upgrade if that would solve things as I got rid of the Deprecated messaging class completely. Your config looks fine to me.

jrochkind commented 5 years ago

It's set to false in a conditional, that branch of the condition was not the active one in the situation where I saw the deprecation warning. I was setting it to the hash.

I could try upgrading. I don't think I see any breaking changes in the upgrade notes, except that you can't use the deprecated initialization I'm not using anyway? So it should be a smooth upgrade?

I wonder if that would somehow get me a total failure on cap deploy failure though, if the underlying problem is that it's somehow not getting the configuration properly in the failure notification case!

I probably just need to create an intentional cap deploy failure to reproduce, and see.

jrochkind commented 5 years ago

Now I'm having trouble reproducing the message, even in a deploy failure. I swear I saw it before, that's the only way I could have copy and pasted it into this issue! I am confused. But will go ahead and update slackistrano to 4.0.1.