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

Re-add file size check prior to reading file #2321

Merged
merged 2 commits into from
Dec 19, 2022
Merged

Re-add file size check prior to reading file #2321

merged 2 commits into from
Dec 19, 2022

Conversation

bethanyj28
Copy link
Contributor

@bethanyj28 bethanyj28 commented Dec 16, 2022

The job hangs when uploading a large summary file since we had moved a file size check to another location. We suspect it has to do with MaskSecrets reading the file, but we will re-add the check to fix the issue for the time being.

Prior to the change:
SCR-20221216-hue

After the change:
SCR-20221216-hul

@bethanyj28 bethanyj28 requested a review from a team as a code owner December 16, 2022 17:47
@bethanyj28 bethanyj28 changed the base branch from main to releases/m300 December 16, 2022 17:58
@@ -182,6 +182,14 @@ public void ProcessCommand(IExecutionContext context, string filePath, Container
return;
}

if (fileSize > AttachmentSizeLimit)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably also remove this block where it is added in the if/else block below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in 674f15b!

Choose a reason for hiding this comment

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

Beautiful!!

@fhammerl fhammerl merged commit ffae5b7 into actions:releases/m300 Dec 19, 2022
@bethanyj28 bethanyj28 deleted the bethanyj28/check-file-size branch December 19, 2022 13:59
@TingluoHuang
Copy link
Member

@bethanyj28 Please cherry-pick the fix back to the main branch.

bethanyj28 added a commit to bethanyj28/runner that referenced this pull request Dec 19, 2022
* Re-add file size check prior to reading file

* Remove redundant file size check
@bethanyj28
Copy link
Contributor Author

@TingluoHuang PR open: #2330

Thanks!

TingluoHuang pushed a commit that referenced this pull request Dec 19, 2022
* Re-add file size check prior to reading file

* Remove redundant file size check
nikola-jokic pushed a commit to nikola-jokic/runner that referenced this pull request May 12, 2023
…2330)

* Re-add file size check prior to reading file

* Remove redundant file size check
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.

5 participants