Skip to content

Conversation

tollsimy
Copy link

@tollsimy tollsimy commented May 4, 2025

This PR complements what added in kernelci/kcidb-io#99 and kernelci/kcidb#635, allowing to send reference to input files to kcidb as well as input files to "artifacts" field in maestro.
This PR requires kernelci/kernelci-core#2872.

@tollsimy tollsimy force-pushed the feat/input-files branch 3 times, most recently from a01d1d5 to 5497db3 Compare May 6, 2025 22:19
@tollsimy tollsimy marked this pull request as ready for review May 6, 2025 22:21
@nuclearcat nuclearcat added the staging-skip Don't test automatically on staging.kernelci.org label May 16, 2025
Copy link

@tales-aparecida tales-aparecida left a comment

Choose a reason for hiding this comment

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

I have some nitpicking, which you can select on your discretion, but consider also that we could, otherwise, rename _get_output_files to something like _filter_artifacts and pass
exclude_properties=[f"input_{i}" for i in INPUT_FILE_TYPES] to avoid adding the duplicated code within _get_input_files

@tollsimy
Copy link
Author

Thanks @tales-aparecida for the suggestions!
I pushed a refactor commit to avoid code duplication. Originally, I kept the same structure mainly for readability and clarity for reviewers, but of course it makes more sense to refactor properly.

@tollsimy tollsimy requested a review from tales-aparecida June 10, 2025 09:44
Copy link

@tales-aparecida tales-aparecida left a comment

Choose a reason for hiding this comment

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

Not to abuse your patience, but here's more nit picking

parsed_test_node['output_files'] = self._get_output_files(
parsed_test_node['input_files'] = self._filter_artifacts(
artifacts=artifacts,
include_properties=('input_')

Choose a reason for hiding this comment

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

Suggested change
include_properties=('input_')
include_properties=('input_',)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
staging-skip Don't test automatically on staging.kernelci.org
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants