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

Add a deliver! version of GenericMailer.deliver #23037

Merged
merged 2 commits into from
May 24, 2024

Conversation

agrare
Copy link
Member

@agrare agrare commented May 15, 2024

Add a version of GenericMailer.deliver which raises exceptions on failure and have the "async" queue/task methods call this. This should allow for us to get an error on the miq_task if delivery failed.

@agrare agrare requested a review from Fryguy as a code owner May 15, 2024 13:31
@agrare agrare changed the title Add a deliver! version of GenericMailer.deliver [WIP] Add a deliver! version of GenericMailer.deliver May 15, 2024
@miq-bot miq-bot added the wip label May 15, 2024
Add a version of `GenericMailer.deliver` which raises exceptions on
failure.
@agrare agrare force-pushed the add_generic_mailer_deliver! branch from f9a6037 to 7c77e60 Compare May 15, 2024 13:47
@miq-bot
Copy link
Member

miq-bot commented May 15, 2024

Checked commits agrare/manageiq@9beea07~...7c77e60 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. ⭐

@agrare agrare changed the title [WIP] Add a deliver! version of GenericMailer.deliver Add a deliver! version of GenericMailer.deliver May 15, 2024
@miq-bot miq-bot removed the wip label May 15, 2024
@Fryguy
Copy link
Member

Fryguy commented May 17, 2024

@miq-bot cross-repo-test manageiq-ui-classic, manageiq-api, manageiq-content, manageiq-automation_engine

@Fryguy Fryguy self-assigned this May 17, 2024
miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request May 17, 2024
miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request May 17, 2024
# catch delivery errors if raised,
rescue Net::SMTPError => e
msg.deliver_now!
rescue Net::SMTPError => e # catch delivery errors if raised
Copy link
Member

Choose a reason for hiding this comment

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

I thought rubocop prefers comments above the line

rescue => e # catch delivery errors if raised,
_log.error("method: #{method} options: #{options} delivery-error #{e}")
end

def self.deliver_queue(method, options = {}, queue_options = {})
Copy link
Member

Choose a reason for hiding this comment

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

I'm assuming this is ok but it is kind of deliver_queue!

@kbrock
Copy link
Member

kbrock commented May 21, 2024

@Fryguy Is this good to go?

@Fryguy
Copy link
Member

Fryguy commented May 21, 2024

cross-repo failed on the automation engine - not sure if related or not

@agrare
Copy link
Member Author

agrare commented May 24, 2024

@miq-bot cross-repo-test manageiq-ui-classic, manageiq-api, manageiq-content, ManageIQ/manageiq-automation_engine#548

miq-bot pushed a commit to ManageIQ/manageiq-cross_repo-tests that referenced this pull request May 24, 2024
@agrare
Copy link
Member Author

agrare commented May 24, 2024

ManageIQ/manageiq-automation_engine#548 fixes the automation_engine failure

@agrare
Copy link
Member Author

agrare commented May 24, 2024

Cross repo is green now

@Fryguy Fryguy merged commit 671a04b into ManageIQ:master May 24, 2024
8 checks passed
@Fryguy
Copy link
Member

Fryguy commented May 24, 2024

Backported to radjabov in commit 719adb5.

commit 719adb594a30b71be77f367a7b7017c44c649e3d
Author: Jason Frey <fryguy9@gmail.com>
Date:   Fri May 24 16:29:58 2024 -0400

    Merge pull request #23037 from agrare/add_generic_mailer_deliver!
    
    Add a `deliver!` version of `GenericMailer.deliver`
    
    (cherry picked from commit 671a04b0685b02d039ead5c9f14cd38c617520f7)

Fryguy added a commit that referenced this pull request May 24, 2024
Add a `deliver!` version of `GenericMailer.deliver`

(cherry picked from commit 671a04b)
@agrare agrare deleted the add_generic_mailer_deliver! branch May 28, 2024 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants