Migrate pushover_notify modules to extension

salt-extensions / extension_migration

0 stars 4 forks source link

Migrate pushover_notify modules to extension #129

Closed garethgreenaway closed 10 months ago

garethgreenaway commented 10 months ago

modules/pushover_notify.py states/pushover.py

lkubb commented 10 months ago

If someone (@nicholasmhughes ?) can assign this issue to me, I would be willing to create an extension for this. Have some fixes as well, afair it's broken as currently present in Salt core.

nicholasmhughes commented 10 months ago

@lkubb , if you can port the current code over as-is first, get that merged, and then create a PR for the changes... that will allow us to cut a "1.0.0" release that is exactly what is currently in Salt. It should be easier to review that way, and also help in troubleshooting any issues between versions. Thanks!

lkubb commented 10 months ago

@nicholasmhughes I migrated the code as-is to https://github.com/lkubb/saltext-pushover, PR for marking it as deprecated will come next

Edit: Just noticed CI failures, sorry for the early ping.

nicholasmhughes commented 10 months ago

Created https://github.com/salt-extensions/saltext-pushover for when you're ready

lkubb commented 10 months ago

Thanks :) It seems the pre-commit crash while building the extension wheel is not unique to saltext-pushover:

error: [Errno 2] No such file or directory: 'build/bdist.linux-x86_64/wheel/saltext/pushover/modules/__init__.py'
error: [Errno 13] Permission denied: 'build/bdist.linux-x86_64/wheel/saltext.saltext_namecheap-0.1.dev1+g41c96eb.d20231031-py3.7.egg-info'
error: [Errno 2] No such file or directory: 'build/bdist.linux-x86_64/wheel/saltext.saltext_mysql-0.1.dev1+ga006706-py3.8.egg-info'

pre-commit runs hooks in parallel and the nox action tries to build and install a wheel, which is racy. Will submit a bug report and try to fix it in the extension template.

lkubb commented 10 months ago

OK, I have now fixed the pre-commit pylint issue as well as the docs build job failing (and again submitted the fixes to the official template).

The only CI jobs still failing are the test runs, and that's because there aren't any tests for the pushover modules in Salt core and pylint exits with 5 in this case. Will push the repo now and submit a deprecation PR, so I think this can be closed.