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

Allow directory of payload files #115

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jcipar
Copy link

@jcipar jcipar commented May 8, 2024

If the payload file is a diretory, load all immediate child files as payloads for the workload.

If the payload file is a diretory, load all immediate child files as
payloads for the workload.
@jcipar jcipar requested a review from a team as a code owner May 8, 2024 20:45
@jcipar jcipar requested review from ballard26 and removed request for a team May 8, 2024 20:45
@CLAassistant
Copy link

CLAassistant commented May 8, 2024

CLA assistant check
All committers have signed the CLA.

@jcipar jcipar requested a review from travisdowns May 8, 2024 20:51
@@ -120,7 +121,15 @@ public TestResult run() throws Exception {
}
}
else {
producerWorkAssignment.payloadData.add(payloadReader.load(workload.payloadFile));
File payloadFile = new File(workload.payloadFile);
Copy link
Member

@travisdowns travisdowns May 8, 2024

Choose a reason for hiding this comment

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

Really good idea to re-use the existing filename field, I'd only add that if it contains no files maybe just assert out with "directory was empty" or smth.

Copy link
Author

Choose a reason for hiding this comment

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

Do you think it's worth it to recursively search the directory? I didn't do it at first just because I wanted to try the idea. I'm not sure what the user would expect.

@jcipar jcipar marked this pull request as draft May 8, 2024 20:51
Copy link
Member

@travisdowns travisdowns left a comment

Choose a reason for hiding this comment

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

minor change requested

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