[Build-trigger-54] Sending a confirmation email to the person that triggered the build #59

khermano · 2024-10-17T14:13:43Z

resolves #54

petrberan · 2024-10-23T08:52:44Z

src/main/java/org/jboss/set/BuildTrigger.java

@@ -27,10 +32,22 @@ public class BuildTrigger {
    public void triggerBuild(BuildJMSTriggerPayload payloadMessage) {
        try (JMSContext context = connectionFactory.createContext(Session.AUTO_ACKNOWLEDGE)) {
            context.createProducer().send(context.createTopic(triggerTopic), objectMapper.writeValueAsString(payloadMessage));
+            logger.infof("Build message successfully sent to UMB for repository: %s, version: %s",


-> "Build message sent to UMB..."

petrberan · 2024-10-23T08:53:19Z

src/main/java/org/jboss/set/BuildTrigger.java

@@ -27,10 +32,22 @@ public class BuildTrigger {
    public void triggerBuild(BuildJMSTriggerPayload payloadMessage) {
        try (JMSContext context = connectionFactory.createContext(Session.AUTO_ACKNOWLEDGE)) {
            context.createProducer().send(context.createTopic(triggerTopic), objectMapper.writeValueAsString(payloadMessage));
+            logger.infof("Build message successfully sent to UMB for repository: %s, version: %s",
+                    payloadMessage.gitRepo, payloadMessage.projectVersion);
+                    if (payloadMessage.email != null && !payloadMessage.email.isBlank()


Is the indentation alright for this if clause?

petrberan · 2024-10-23T08:54:39Z

src/main/java/org/jboss/set/BuildTrigger.java

+        } catch (JMSRuntimeException e) {
+            logger.errorf("Failed to send build message to UMB for tag: %s. Exception: %s",
+                    payloadMessage.tag, e.getMessage());
+            throw new BuildTriggerException("Failed to send build message to UMB due to internal error: " + e.getMessage(), e);


-> "Failed to send build message to UMB due to: ..."

petrberan · 2024-10-23T08:56:50Z

src/main/java/org/jboss/set/BuildTrigger.java

-            throw new RuntimeException("Failed to serialize BuildJMSTriggerPayload to JSON: " + e.getMessage(), e);
+            throw new BuildTriggerException("Failed to serialize BuildJMSTriggerPayload to JSON: " + e.getMessage(), e);
+        } catch (JMSRuntimeException e) {
+            logger.errorf("Failed to send build message to UMB for tag: %s. Exception: %s",


We have two similar loggers, one on L35, saying that the build was sent and the other here, saying that it wasn't sent. Both of them are including different information. Should be unified

petrberan · 2024-10-23T08:59:05Z

src/main/java/org/jboss/set/EmailService.java

+    Mailer mailer;
+
+    public void sendMail(BuildJMSTriggerPayload payload) {
+        String subject = "Request to build project: " + payload.gitRepo + ", version: " + payload.projectVersion + " successfully sent";


That's a way too long for a subject. I'd go with something like [Build Trigger] Build of <component>, version <version> requested"

petrberan · 2024-10-23T09:14:49Z

src/main/java/org/jboss/set/EmailService.java

+
+    public void sendMail(BuildJMSTriggerPayload payload) {
+        String subject = "Request to build project: " + payload.gitRepo + ", version: " + payload.projectVersion + " successfully sent";
+        String text = "Request to build project: " + payload.gitRepo + ", version: " + payload.projectVersion


Request to build <component> in version <version> was successfully sent to a productization team using following parameters:

Tag:
Repository:
Version:
Commit:
Streams:

Report any issues or share suggestions using our GitHub Repository

This is an automated Build Trigger message, please do not respond!

petrberan · 2024-10-23T09:16:26Z

src/main/java/org/jboss/set/EmailService.java

+                .setText(text)
+                .setTo(List.of(payload.email)));
+
+        logger.infof("Sending mail to %s about successful request to build project.\n Subject: %s\n Text: %s", payload.email, subject, text);


This is perhaps way too much logging given the fact that we can check the email itself.

logger.infof("Sending mail to %s about successful request to build project", payload.email);

I moved this to BuildTrigger and modified it a bit because we are not the ones receiving the email. The person who asked for a build is receiving the email and we should get at least a notification about sending this email so we know everything went well. Or what did you mean?

I meant that the content of the email can be verified either by sending the email to us or said person can redirect the email to us as well

OK, I removed that.

petrberan · 2024-10-23T09:17:07Z

src/main/java/org/jboss/set/model/json/BuildJMSTriggerPayload.java

-                ", version='" + projectVersion + '\'' +
-                ", commitSha='" + commitSha + '\'' +
-                ", streams='" + streams + '\'' +
+        return "BuildJMSTriggerPayload{" + "\n" +


The email would probably require some formatting method to somewhat prettify the data here

petrberan · 2024-10-24T06:02:06Z

src/main/java/org/jboss/set/EmailService.java

+    Mailer mailer;
+
+    public void sendMail(BuildJMSTriggerPayload payload) {
+        String subject = "[Build Trigger] Build of " + payload.gitRepo + ", version: " + payload.projectVersion + " requested";


That's not a component, but a git repo :) The line (as well as the one below) should look like "[Build Trigger] Build of Resteasy, version 3.2.1 requested" as opposed to "[Build Trigger] Build of https://github.com/resteasy/Resteasy/, version 3.2.1 requested"

Or similar formatting, looking at it I'm not a fan of the ",version x.x.x requested" part. Do you have any idea how to change it?

We might want to add project property for the BuildInfo as we'll be using it here and in the CU creation as well. For now, let's add

String project = payload.gitRepo.substring(payload.gitRepo.lastIndexOf('/') + 1)

…iggered the build

xjusko

LGTM, thanks

khermano force-pushed the build-trigger-54_from_main branch from da9472d to 63f66bc Compare October 23, 2024 08:27

petrberan reviewed Oct 23, 2024

View reviewed changes

petrberan requested a review from xjusko October 23, 2024 11:30

khermano force-pushed the build-trigger-54_from_main branch 3 times, most recently from 7e52126 to 3fb2fbf Compare October 23, 2024 13:20

petrberan reviewed Oct 24, 2024

View reviewed changes

[Build-trigger-54] Sending a confirmation email to the person that tr…

b0e440f

…iggered the build

khermano force-pushed the build-trigger-54_from_main branch from 3fb2fbf to b0e440f Compare October 24, 2024 07:54

xjusko approved these changes Oct 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Build-trigger-54] Sending a confirmation email to the person that triggered the build #59

[Build-trigger-54] Sending a confirmation email to the person that triggered the build #59

khermano commented Oct 17, 2024

petrberan Oct 23, 2024

petrberan Oct 23, 2024

petrberan Oct 23, 2024

petrberan Oct 23, 2024

petrberan Oct 23, 2024 •

edited

Loading

petrberan Oct 23, 2024

petrberan Oct 23, 2024

khermano Oct 23, 2024

petrberan Oct 23, 2024

khermano Oct 23, 2024

petrberan Oct 23, 2024

petrberan Oct 24, 2024

xjusko left a comment

[Build-trigger-54] Sending a confirmation email to the person that triggered the build #59

Are you sure you want to change the base?

[Build-trigger-54] Sending a confirmation email to the person that triggered the build #59

Conversation

khermano commented Oct 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petrberan Oct 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xjusko left a comment

Choose a reason for hiding this comment

petrberan Oct 23, 2024 •

edited

Loading