Skip to content

Add retries for Build image in Azure#577

Merged
motm32 merged 6 commits intomainfrom
meganmott/fix-562
Jan 22, 2024
Merged

Add retries for Build image in Azure#577
motm32 merged 6 commits intomainfrom
meganmott/fix-562

Conversation

@motm32
Copy link
Copy Markdown
Contributor

@motm32 motm32 commented Dec 5, 2023

Fixes #562.

I arbitrarily chose 3 retries since that should cover the timing issue and building the image takes a bit of time. Let me know if anyone disagrees with that choice.

I also tried a couple of times to test the retries but I could only get it to work on the first try. Guess our code is just too good 🤪

@motm32 motm32 requested a review from a team as a code owner December 5, 2023 19:42
Comment thread src/commands/image/imageSource/buildImageInAzure/RunStep.ts
Comment thread src/commands/image/imageSource/buildImageInAzure/RunStep.ts Outdated
Copy link
Copy Markdown
Member

@nturinski nturinski left a comment

Choose a reason for hiding this comment

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

Is there a way for us to parse the error that's coming from the logs? I remember that there was a specific error about the url being incorrect that Anthony was running into.

I'm think that we may want to check for that error specifically because otherwise the retry logic might be annoying if it's failing for totally different reasonsl

@MicroFish91
Copy link
Copy Markdown
Contributor

MicroFish91 commented Dec 8, 2023

Is there a way for us to parse the error that's coming from the logs? I remember that there was a specific error about the url being incorrect that Anthony was running into.

I'm think that we may want to check for that error specifically because otherwise the retry logic might be annoying if it's failing for totally different reasonsl

Yeah that's a good idea. I know we talked briefly about seeing if we could parse errors from the logs for other reasons as well. Would it be worth it for us to reach out to Julius' team to get a better understanding about how the log messages are formatted under different error conditions?

@jinglouMSFT
Copy link
Copy Markdown

Have we done a test run on how long it takes to retry 3 times? while retrying, do we show any UI indications?

@MicroFish91
Copy link
Copy Markdown
Contributor

MicroFish91 commented Jan 5, 2024

Have we done a test run on how long it takes to retry 3 times? while retrying, do we show any UI indications?

I believe Megan is showing some UI indications for each attempt in the activity log.

I've tried testing it with a bad URL a few times and it seems to complete all retries in around or a little bit less than 10 seconds. We may want to have Anthony test it later to see if we need to adjust the number of retries.

Is there a way for us to parse the error that's coming from the logs? I remember that there was a specific error about the url being incorrect that Anthony was running into.

I was looking into this issue with Megan, and I think we might not even need to parse the logs after all. The error seems to throw specifically when the url is incorrect/not ready yet.

If, say, the run fails due to something else like the Dockerfile, it will not throw right away in the run step because the run merely starts there, and until we poll its status again during BuildImageStep, we won't know whether or not the build failed. So basically, if we see an error throw, it's probably safe to assume the URL was the issue without any additional parsing whatsoever.

@motm32
Copy link
Copy Markdown
Contributor Author

motm32 commented Jan 8, 2024

Yes Matt is correct the Activity log does show an indication of each attempt like this:
image

The Azure Container Apps logs also contain this information:
image
I also checked and the retries also only look about 10 seconds to complete for me.

Comment thread src/commands/image/imageSource/buildImageInAzure/RunStep.ts
ext.outputChannel.appendLog(message);

context.run = await context.client.registries.beginScheduleRunAndWait(context.resourceGroupName, context.registryName, runRequest);
runRequest.sourceLocation = 'a'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's this all about?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is an oopsie by me. I was using this to make the run fail.

@motm32 motm32 merged commit 3270794 into main Jan 22, 2024
@motm32 motm32 deleted the meganmott/fix-562 branch January 22, 2024 18:17
@microsoft microsoft locked and limited conversation to collaborators Mar 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add retries when executing Build Image in Azure

4 participants