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

cli: remove ssl cert warning #2303

Merged
merged 1 commit into from
Jun 11, 2022
Merged

cli: remove ssl cert warning #2303

merged 1 commit into from
Jun 11, 2022

Conversation

half-duplex
Copy link
Member

@half-duplex half-duplex commented Jun 9, 2022

Description

I missed this in #2278 😐
Since ca_certs=None is now valid, the warning is nonsense. If the setting is truly invalid, Sopel will quit with an exception:

ValueError: Invalid value for core.ca_certs: Value must be an existing or creatable file.

or

ssl.SSLError: [X509: NO_CERTIFICATE_OR_CRL_FOUND] no certificate or crl found (_ssl.c:4123)

(Though after the latter, it'll still attempt to reconnect after shutting down??)

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

@half-duplex half-duplex added the Bugfix Generally, PRs that reference (and fix) one or more issue(s) label Jun 9, 2022
@half-duplex half-duplex added this to the 8.0.0 milestone Jun 9, 2022
@dgw
Copy link
Member

dgw commented Jun 9, 2022

(Though after the latter, it'll still attempt to reconnect after shutting down??)

Some weird lifecycle stuff happening, maybe. That should be at least caught, but attrs like hasquit are still a bit wonky. Good for another patch if you fall into that rabbit hole.

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe #2306 touches some stuff that will fix trying to reconnect after SSL failure, so there's no need to block this on it.

@dgw dgw merged commit d6c28d4 into sopel-irc:master Jun 11, 2022
@half-duplex half-duplex deleted the ssl-warning branch June 11, 2022 18:27
@half-duplex
Copy link
Member Author

(Also, this doesn't affect the result, just removes the nonsense message preceding it)

@dgw
Copy link
Member

dgw commented Jun 11, 2022

(Should it be relabeled as a Tweak or Housekeeping instead of Bugfix?)

@half-duplex
Copy link
Member Author

I figure telling the user something outright incorrect counts as a bug. Not quitting is more or less unrelated.

@dgw dgw added Housekeeping Code cleanup, removal of deprecated stuff, etc. and removed Bugfix Generally, PRs that reference (and fix) one or more issue(s) labels Jun 11, 2022
@dgw
Copy link
Member

dgw commented Jun 11, 2022

Exirel agreeing with me on IRC was enough to push me into "this isn't a bug" territory. (More of a record-keeping thing than any kind of defense. Just didn't like leaving the thread dangling like that after the label change.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping Code cleanup, removal of deprecated stuff, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants