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

[tmsUpload, tmsExport] Provide additional log message on successful upload and export to node #4624

4 changes: 2 additions & 2 deletions cmd/tmsExport.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func tmsExport(exportConfig tmsExportOptions, telemetryData *telemetry.CustomDat

func runTmsExport(exportConfig tmsExportOptions, communicationInstance tms.CommunicationInterface, utils tms.TmsUtils) error {
config := convertExportOptions(exportConfig)
fileId, errUploadFile := tms.UploadFile(config, communicationInstance, utils)
fileInfo, errUploadFile := tms.UploadFile(config, communicationInstance, utils)
if errUploadFile != nil {
return errUploadFile
}
Expand All @@ -38,7 +38,7 @@ func runTmsExport(exportConfig tmsExportOptions, communicationInstance tms.Commu
return errUploadDescriptors
}

_, errExportFileToNode := communicationInstance.ExportFileToNode(config.NodeName, fileId, config.CustomDescription, config.NamedUser)
_, errExportFileToNode := communicationInstance.ExportFileToNode(fileInfo, config.NodeName, config.CustomDescription, config.NamedUser)
o-liver marked this conversation as resolved.
Show resolved Hide resolved
if errExportFileToNode != nil {
log.SetErrorCategory(log.ErrorService)
return fmt.Errorf("failed to export file to node: %w", errExportFileToNode)
Expand Down
2 changes: 1 addition & 1 deletion cmd/tmsExport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func newTmsExportTestsUtils() tmsExportMockUtils {
return utils
}

func (cim *communicationInstanceMock) ExportFileToNode(nodeName, fileId, description, namedUser string) (tms.NodeUploadResponseEntity, error) {
func (cim *communicationInstanceMock) ExportFileToNode(fileInfo FileInfo, nodeName, description, namedUser string) (tms.NodeUploadResponseEntity, error) {
var nodeUploadResponseEntity tms.NodeUploadResponseEntity
if description != CUSTOM_DESCRIPTION || nodeName != NODE_NAME || fileId != strconv.FormatInt(FILE_ID, 10) || namedUser != NAMED_USER {
return nodeUploadResponseEntity, errors.New(INVALID_INPUT_MSG)
Expand Down
4 changes: 2 additions & 2 deletions cmd/tmsUpload.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func tmsUpload(uploadConfig tmsUploadOptions, telemetryData *telemetry.CustomDat

func runTmsUpload(uploadConfig tmsUploadOptions, communicationInstance tms.CommunicationInterface, utils tms.TmsUtils) error {
config := convertUploadOptions(uploadConfig)
fileId, errUploadFile := tms.UploadFile(config, communicationInstance, utils)
fileInfo, errUploadFile := tms.UploadFile(config, communicationInstance, utils)
if errUploadFile != nil {
return errUploadFile
}
Expand All @@ -31,7 +31,7 @@ func runTmsUpload(uploadConfig tmsUploadOptions, communicationInstance tms.Commu
return errUploadDescriptors
}

_, errUploadFileToNode := communicationInstance.UploadFileToNode(config.NodeName, fileId, config.CustomDescription, config.NamedUser)
_, errUploadFileToNode := communicationInstance.UploadFileToNode(fileInfo, config.NodeName, config.CustomDescription, config.NamedUser)
if errUploadFileToNode != nil {
log.SetErrorCategory(log.ErrorService)
return fmt.Errorf("failed to upload file to node: %w", errUploadFileToNode)
Expand Down
2 changes: 1 addition & 1 deletion cmd/tmsUpload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (cim *communicationInstanceMock) UploadFile(file, namedUser string) (tms.Fi
}
}

func (cim *communicationInstanceMock) UploadFileToNode(nodeName, fileId, description, namedUser string) (tms.NodeUploadResponseEntity, error) {
func (cim *communicationInstanceMock) UploadFileToNode(fileInfo FileInfo, nodeName, description, namedUser string) (tms.NodeUploadResponseEntity, error) {
var nodeUploadResponseEntity tms.NodeUploadResponseEntity
if description != CUSTOM_DESCRIPTION || nodeName != NODE_NAME || fileId != strconv.FormatInt(FILE_ID, 10) || namedUser != NAMED_USER {
return nodeUploadResponseEntity, errors.New(INVALID_INPUT_MSG)
Expand Down
24 changes: 16 additions & 8 deletions pkg/tms/tmsClient.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/http"
"net/url"
"os"
"strconv"
"strings"

piperHttp "github.com/SAP/jenkins-library/pkg/http"
Expand Down Expand Up @@ -177,7 +178,9 @@ func (communicationInstance *CommunicationInstance) GetMtaExtDescriptor(nodeId i

}

func (communicationInstance *CommunicationInstance) UploadFileToNode(nodeName, fileId, description, namedUser string) (NodeUploadResponseEntity, error) {
func (communicationInstance *CommunicationInstance) UploadFileToNode(fileInfo FileInfo, nodeName, description, namedUser string) (NodeUploadResponseEntity, error) {
fileId := strconv.FormatInt(fileInfo.Id, 10)

if communicationInstance.isVerbose {
communicationInstance.logger.Info("Node upload started")
communicationInstance.logger.Infof("tmsUrl: %v, nodeName: %v, fileId: %v, description: %v, namedUser: %v", communicationInstance.tmsUrl, nodeName, fileId, description, namedUser)
Expand All @@ -200,14 +203,18 @@ func (communicationInstance *CommunicationInstance) UploadFileToNode(nodeName, f
}

json.Unmarshal(data, &nodeUploadResponseEntity)
if communicationInstance.isVerbose {
communicationInstance.logger.Info("Node upload executed successfully")
}
communicationInstance.logger.Info("Node upload executed successfully")

// Important: there are Customers, who might rely on format of this log message to parse transport request id
Copy link
Member

Choose a reason for hiding this comment

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

And we cannot offer them something else, such as variables in the common pipeline environment (CPE)? Maybe additionally to the log message we could also provide the information in CPE variables. Then they can use that in the pipeline, without having to parse the log.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And we cannot offer them something else, such as variables in the common pipeline environment (CPE)?

Yes, that will be an ideal case. For now, we have agreed with the Customer, who complained about missing log message after we switched to the new Golang implementation of the step, that we provide a log message with transport request id in the Golang implementation as well.

Usage of variables in CPE or other options, if they are, have to be and will be discussed separately.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, let's do so. @marcusholl What do you think? Would it help Piper users, if they could retrieve this information from the CPE, rather than parsing the log? Is there something else we could offer, apart from the CPE?

communicationInstance.logger.Infof("nodeName: %v, nodeId: %v, uploadedFile: %v, createdTransportRequestDescription: %v, createdTransportRequestId: %v", nodeUploadResponseEntity.QueueEntries[0].NodeName, nodeUploadResponseEntity.QueueEntries[0].NodeId, fileInfo.Name, nodeUploadResponseEntity.TransportRequestDescription, nodeUploadResponseEntity.TransportRequestId)

return nodeUploadResponseEntity, nil

}

func (communicationInstance *CommunicationInstance) ExportFileToNode(nodeName, fileId, description, namedUser string) (NodeUploadResponseEntity, error) {
func (communicationInstance *CommunicationInstance) ExportFileToNode(fileInfo FileInfo, nodeName, description, namedUser string) (NodeUploadResponseEntity, error) {
fileId := strconv.FormatInt(fileInfo.Id, 10)

if communicationInstance.isVerbose {
communicationInstance.logger.Info("Node export started")
communicationInstance.logger.Infof("tmsUrl: %v, nodeName: %v, fileId: %v, description: %v, namedUser: %v", communicationInstance.tmsUrl, nodeName, fileId, description, namedUser)
Expand All @@ -230,9 +237,10 @@ func (communicationInstance *CommunicationInstance) ExportFileToNode(nodeName, f
}

json.Unmarshal(data, &nodeUploadResponseEntity)
if communicationInstance.isVerbose {
communicationInstance.logger.Info("Node export executed successfully")
}
communicationInstance.logger.Info("Node export executed successfully")

// Important: there are Customers, who might rely on format of this log message to parse transport request id
communicationInstance.logger.Infof("nodeName: %v, nodeId: %v, uploadedFile: %v, createdTransportRequestDescription: %v, createdTransportRequestId: %v", nodeUploadResponseEntity.QueueEntries[0].NodeName, nodeUploadResponseEntity.QueueEntries[0].NodeId, fileInfo.Name, nodeUploadResponseEntity.TransportRequestDescription, nodeUploadResponseEntity.TransportRequestId)
return nodeUploadResponseEntity, nil

}
Expand Down
8 changes: 4 additions & 4 deletions pkg/tms/tmsClient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,9 +469,9 @@ func TestUploadFileToNode(t *testing.T) {

communicationInstance := CommunicationInstance{tmsUrl: "https://tms.dummy.sap.com", httpClient: &uploaderMock, logger: logger, isVerbose: false}

fileId := "111"
fileInfo := FileInfo{Id: "111", Name: "test.mtar"}
namedUser := "testUser"
nodeUploadResponseEntity, err := communicationInstance.UploadFileToNode(nodeName, fileId, transportRequestDescription, namedUser)
nodeUploadResponseEntity, err := communicationInstance.UploadFileToNode(fileInfo, nodeName, transportRequestDescription, namedUser)

assert.NoError(t, err, "Error occurred, but none expected")
assert.Equal(t, "https://tms.dummy.sap.com/v2/nodes/upload", uploaderMock.urlCalled, "Called url incorrect")
Expand All @@ -493,11 +493,11 @@ func TestUploadFileToNode(t *testing.T) {
uploaderMock := uploaderMock{responseBody: `Bad request provided`, httpStatusCode: http.StatusBadRequest}
communicationInstance := CommunicationInstance{tmsUrl: "https://tms.dummy.sap.com", httpClient: &uploaderMock, logger: logger, isVerbose: false}

fileInfo := FileInfo{Id: "111", Name: "test.mtar"}
nodeName := "TEST_NODE"
fileId := "111"
transportRequestDescription := "This is a test description"
namedUser := "testUser"
_, err := communicationInstance.UploadFileToNode(nodeName, fileId, transportRequestDescription, namedUser)
_, err := communicationInstance.UploadFileToNode(fileInfo, nodeName, transportRequestDescription, namedUser)

assert.Error(t, err, "Error expected, but none occurred")
assert.Equal(t, "https://tms.dummy.sap.com/v2/nodes/upload", uploaderMock.urlCalled, "Called url incorrect")
Expand Down
16 changes: 8 additions & 8 deletions pkg/tms/tmsUtils.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"net/url"
"sort"
"strconv"

"github.com/SAP/jenkins-library/pkg/command"
piperHttp "github.com/SAP/jenkins-library/pkg/http"
Expand Down Expand Up @@ -101,8 +100,8 @@ type CommunicationInterface interface {
UpdateMtaExtDescriptor(nodeId, idOfMtaExtDescriptor int64, file, mtaVersion, description, namedUser string) (MtaExtDescriptor, error)
UploadMtaExtDescriptorToNode(nodeId int64, file, mtaVersion, description, namedUser string) (MtaExtDescriptor, error)
UploadFile(file, namedUser string) (FileInfo, error)
UploadFileToNode(nodeName, fileId, description, namedUser string) (NodeUploadResponseEntity, error)
ExportFileToNode(nodeName, fileId, description, namedUser string) (NodeUploadResponseEntity, error)
UploadFileToNode(fileInfo FileInfo, nodeName, description, namedUser string) (NodeUploadResponseEntity, error)
ExportFileToNode(fileInfo FileInfo, nodeName, description, namedUser string) (NodeUploadResponseEntity, error)
}

type Options struct {
Expand Down Expand Up @@ -339,20 +338,21 @@ func UploadDescriptors(config Options, communicationInstance CommunicationInterf
return nil
}

func UploadFile(config Options, communicationInstance CommunicationInterface, utils TmsUtils) (string, error) {
func UploadFile(config Options, communicationInstance CommunicationInterface, utils TmsUtils) (FileInfo, error) {
var fileInfo FileInfo

mtaPath := config.MtaPath
exists, _ := utils.FileExists(mtaPath)
if !exists {
log.SetErrorCategory(log.ErrorConfiguration)
return "", fmt.Errorf("mta file %s not found", mtaPath)
return fileInfo, fmt.Errorf("mta file %s not found", mtaPath)
}

fileInfo, errUploadFile := communicationInstance.UploadFile(mtaPath, config.NamedUser)
if errUploadFile != nil {
log.SetErrorCategory(log.ErrorService)
return "", fmt.Errorf("failed to upload file: %w", errUploadFile)
return fileInfo, fmt.Errorf("failed to upload file: %w", errUploadFile)
}

fileId := strconv.FormatInt(fileInfo.Id, 10)
return fileId, nil
return fileInfo, nil
}