Skip to content

Conversation

@DavidGoldwasser
Copy link
Collaborator

@DavidGoldwasser DavidGoldwasser commented Oct 18, 2024

Pull request overview

We no longer want to limit file sizes that go into datapoint zip generated when a workflow is run.

Pull Request Author

@DavidGoldwasser

  • Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
  • All new and existing tests passes

Labels:

  • If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a Code Review on GitHub
  • Code Style, strip trailing whitespace, etc.
  • All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
  • Labeling is ok
  • If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If feature, test running new feature, try creative ways to break it
  • CI status: all green or justified

Copy link
Collaborator

@joseph-robertson joseph-robertson left a comment

Choose a reason for hiding this comment

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

I don't believe I'm familiar with this utility, so it's hard for me to say whether this approach looks good. Does zipDirectory get executed for every workflow run? Should we, at a minimum, set label for this PR to "Pull Request - Ready for CI" so we can check builds are successful?

@DavidGoldwasser
Copy link
Collaborator Author

There should be a data_point.zip file made on every workflow run in the run directory

@wenyikuang wenyikuang force-pushed the issue-5274-remove-zip-size-limits branch from 794c0ce to 77d1ef3 Compare October 29, 2024 03:26
@wenyikuang wenyikuang added the Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. label Oct 29, 2024
@wenyikuang
Copy link
Contributor

Rebased on develop let's run CI and see.

@jmarrec jmarrec changed the title fix issue#5274 so workflow does not remove contents from zip because … Fix #5274 - workflow should not remove contents from zip because of size limit Oct 29, 2024
Comment on lines -249 to -259
auto directorySize = [](const openstudio::path& dirPath) {
uintmax_t dirSize = 0;
for (const auto& dirEnt : fs::recursive_directory_iterator{dirPath}) {
const auto& dirEntryPath = dirEnt.path();
if (fs::is_regular_file(dirEntryPath)) {
dirSize += fs::file_size(dirEntryPath);
}
}
return dirSize;
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

Lambda is now unused, so delete it too

@jmarrec jmarrec merged commit cae9435 into develop Oct 30, 2024
@jmarrec jmarrec deleted the issue-5274-remove-zip-size-limits branch October 30, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component - Workflow Enhancement Request Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. severity - Major Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove check for zip file size from workflow

6 participants