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.

Sign up for GitHub

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

NOISSUE -Rename error status to warning #5

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+15 −15
Open

NOISSUE -Rename error status to warning #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3abf96
rename error to warning
WashingtonKK Sep 20, 2024
a7b10cf
update logging package
WashingtonKK Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions agent/algorithm/logging.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ var (
)

const (
bufSize = 1024
algorithmRun = "AlgorithmRun"
errorStatus = "Error"
bufSize = 1024
algorithmRun = "AlgorithmRun"
warningStatus = "Warning"
)

type Stdout struct {
Expand Down Expand Up @@ -70,7 +70,7 @@ func (s *Stderr) Write(p []byte) (n int, err error) {
s.Logger.Error(string(buf[:n]))
}

if err := s.EventSvc.SendEvent(algorithmRun, errorStatus, json.RawMessage{}); err != nil {
if err := s.EventSvc.SendEvent(algorithmRun, warningStatus, json.RawMessage{}); err != nil {
return len(p), err
}

Expand Down
2 changes: 1 addition & 1 deletion agent/algorithm/logging_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func TestStderrWrite(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mockEventService := mocks.NewService(t)
mockEventService.On("SendEvent", "AlgorithmRun", manager.Error.String(), mock.Anything).Return(nil)
mockEventService.On("SendEvent", "AlgorithmRun", manager.Warning.String(), mock.Anything).Return(nil)

stderr := &Stderr{Logger: mglog.NewMock(), EventSvc: mockEventService}
n, err := stderr.Write([]byte(tt.input))
Expand Down
2 changes: 1 addition & 1 deletion agent/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const (
Ready
Completed
Terminated
Error
Warning
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Inconsistent renaming of 'Error' to 'Warning' across the codebase

The renaming of the 'Error' constant to 'Warning' has not been consistently applied throughout the codebase. While the change has been made in the agent package, it's still present as 'Error' in the manager package. This inconsistency may lead to confusion and potential bugs.

Key points:

  • The file pkg/manager/manager_states.go still uses Error in the ManagerStatus enum.
  • The Warning constant is present in agent/state.go and agent/status_string.go.

Recommendations:

  1. Update pkg/manager/manager_states.go to use Warning instead of Error in the ManagerStatus enum.
  2. Conduct a thorough review of all status-related constants across the codebase to ensure consistency between the manager and agent packages.
  3. Update any related documentation or comments to reflect the change from 'Error' to 'Warning'.

Please address these inconsistencies to maintain code clarity and prevent potential issues arising from mismatched status constants.

Analysis chain

Constant renaming looks good, but verify the impact on the codebase.

The renaming of the constant from Error to Warning aligns with the PR objective and does not introduce any issues in this file.

However, please verify that this change does not adversely affect other parts of the codebase. Ensure that:

  • All references to the Error constant have been updated to Warning.
  • The logic that handles the "warning" state is consistent with the expected behavior.
  • Any relevant documentation or comments have been updated to reflect this change.

You can use the following script to search for references to the Error constant in .go files:

To gather more information about the inconsistency we've found, let's examine the content of pkg/manager/manager_states.go and search for any occurrences of the Warning constant.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for references to the `Error` constant in `.go` files
rg --type go $'\\bError\\b'

Length of output: 11574

Script:

#!/bin/bash
# Display the content of pkg/manager/manager_states.go
echo "Content of pkg/manager/manager_states.go:"
cat pkg/manager/manager_states.go

# Search for occurrences of the 'Warning' constant
echo -e "\nOccurrences of 'Warning' constant:"
rg --type go '\bWarning\b'

Length of output: 720

)

type event uint8
Expand Down
6 changes: 3 additions & 3 deletions agent/status_string.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion manager/vm/logging.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (s *Stderr) Write(p []byte) (n int, err error) {
EventType: manager.VmRunning.String(),
Timestamp: timestamppb.Now(),
Originator: "manager",
Status: manager.Error.String(),
Status: manager.Warning.String(),
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion manager/vm/logging_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func TestStderrWrite(t *testing.T) {
assert.NotNil(t, agentEvent)
assert.Equal(t, "test-computation", agentEvent.ComputationId)
assert.Equal(t, manager.VmRunning.String(), agentEvent.EventType)
assert.Equal(t, manager.Error.String(), agentEvent.Status)
assert.Equal(t, manager.Warning.String(), agentEvent.Status)
assert.NotNil(t, agentEvent.Timestamp)
}
case <-time.After(time.Second):
Expand Down
2 changes: 1 addition & 1 deletion pkg/manager/manager_states.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ type ManagerStatus uint8
const (
Starting ManagerStatus = iota
Stopped
Error
Warning
Disconnected
)
6 changes: 3 additions & 3 deletions pkg/manager/managerstatus_string.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.