Skip to content

Enable 1P Events for VMApps#72

Open
norakoiralamsft wants to merge 9 commits intoAzure:masterfrom
norakoiralamsft:norakoirala/enableappevents
Open

Enable 1P Events for VMApps#72
norakoiralamsft wants to merge 9 commits intoAzure:masterfrom
norakoiralamsft:norakoirala/enableappevents

Conversation

@norakoiralamsft
Copy link
Copy Markdown
Collaborator

No description provided.

@D1v38om83r
Copy link
Copy Markdown
Collaborator

Please remove everything under bundle from the commits. bundle should already be part of .gitignore. We don't want to check-in compilation/build outputs.

return bytes.Equal(checkSumNew, checkSum), nil
}

func logApplicationEvents(downloadDir string, appName string, actionPerformed packageregistry.ActionEnum, errorMessageToReturn error, eem *extensionevents.ExtensionEventManager, actionPlan *ActionPlan) () {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should definitely write any issues with application events to our own extension log, but we should never fail an operation due to this.

stderrFileName := filepath.Join(downloadDir, "stderr")
stderrFile, err := os.Open(stderrFileName)
if err != nil {
errorMessageToReturn = extensionerrors.CombineErrors(errorMessageToReturn, errors.Wrapf(err, "Error opening std err file for application %s", appName))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please double check that CombineErrors is appropriate here - since I believe that returns an ErrorWIthClarification, which we may not want here since it won't have a clarification.

assert.NoError(t, err, "operation should not throw error")
}

func TestPackageRegistryDeserialization_EnableAppEvents_DefaultToFalse(t *testing.T) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tests to verify the actual logging of stderr and stdout?

  • Exceeding buffer size
  • Exceeding event count
  • Etc

@D1v38om83r
Copy link
Copy Markdown
Collaborator

bundle/windows/prod/vm-application-manager.zip is a part of this change. It should already be a part of .gitignore, and we no longer need to check-in this file to the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants