Closed hraftery closed 10 months ago
This could probably use a better test as well. I wonder if there is a way to install Slack in GitHub Actions?
@hraftery as I don't use Slack, this is difficult for me to test. Can you make a PR with the proposed changes after you've confirmed that it's working?
Bump.
This issue is pending a response and is blocked until I have more info. To keep the issue list groomed, I will often close these stalled issues if they go a prolonged time without a response. In this case, I will give it another week or so.
Thanks for the bump. I took a post-xmas moment to reload the brain with this issue.
Turns out you no longer get a "TypeError", because it's now caught earlier and presents a far friendly error message: "ValueError: Could not find a password...". But the underlying issue is the same, so I've tidied up my fix and pushed it (PR coming).
I had a look at test coverage and found test_slack_config()
already catches this error. The challenge, as you imply, is that the test will never pass unless Slack is installed (and, I would guess, run and logged in at least once).
Thank you!
My Issue
The addition I made (adding "Slack" to the list of support browsers) has stopped working, on macOS at least. The error seems mysterious:
WHYT
Turns out the root issue is way back at this line:
"my_pass": keyring.get_password("{} Safe Storage".format(browser), browser)
which seems to quietly (without even prompting for a password) return nothing if the second parameter doesn't have a match.
keyring
calls the parametersservice_name
andusername
but in Keychain Access at least they seem to be called "Name" and "Account".Anyway, point is the "Account" seems to have changed from "Slack" to "Slack Key". That's it!
So a hacky fix is to change the line to something like:
"my_pass": keyring.get_password("{} Safe Storage".format(browser), "Slack Key" if browser == "Slack" else browser)
but I'll have to think a bit more about actually doing it properly.