Skip to content

Test subcategory support(DO NOT MERGE)#97

Draft
VaniHaripriya wants to merge 3 commits intokubeflow:mainfrom
VaniHaripriya:test-subcategory-support
Draft

Test subcategory support(DO NOT MERGE)#97
VaniHaripriya wants to merge 3 commits intokubeflow:mainfrom
VaniHaripriya:test-subcategory-support

Conversation

@VaniHaripriya
Copy link
Copy Markdown
Contributor

DO NO MERGE
This a test PR created to see the component and pipeline generation under subcategories.
Created a component and pipeline under subcategories using the following commands-
make component CATEGORY=training SUBCATEGORY=sklearn_models NAME=logistic_regression CREATE_SHARED=true
make pipeline CATEGORY=training SUBCATEGORY=ml_workflows NAME=batch_training CREATE_SHARED=true

Signed-off-by: Vani Haripriya Mudadla <vmudadla@redhat.com>
Signed-off-by: Vani Haripriya Mudadla <vmudadla@redhat.com>
Signed-off-by: Vani Haripriya Mudadla <vmudadla@redhat.com>
@google-oss-prow
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign humairak for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

done
if [[ $found_assets -eq 0 ]]; then
print_error "'$target_dir' does not contain a $ASSET_FILE file and has no subdirectories with one"
exit 2
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

exit code 2 doesn't seem right here:

2	Misuse of shell builtins	Incorrect usage of a shell built-in command or permission problem.

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.

Updated the exit codes in the main PR -#94

make pipeline CATEGORY=training SUBCATEGORY=ml_workflows NAME=batch_training

# Create pipeline in subcategory with shared utilities package
make pipeline CATEGORY=training SUBCATEGORY=ml_workflows NAME=inference CREATE_SHARED=true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is confusing, because the name is different here than line 412 and 428, can we rename it to batch_training?

make component CATEGORY=training SUBCATEGORY=sklearn_trainer NAME=logistic_regression

# Create component in subcategory with shared utilities package
make component CATEGORY=training SUBCATEGORY=sklearn_trainer NAME=random_forest CREATE_SHARED=true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

will this work, if the subcategory does not exit or in case it already exists?

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.

Yes, it works in both cases. When the subcategory doesn't exist, it creates the full directory structure with OWNERS, README.md, init.py, and optionally shared/. When the subcategory already exists, it skips existing files and only creates the new component inside it. Existing subcategory files are never overwritten.


```bash
# 1. Create feature branch
git checkout -b component/sklearn-logistic-regression upstream/main
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we have to assume that the component exist in a branch here? The component under a subcategory can also be in main branch when someone is trying to contribute

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.

Updated the documentation in the main PR -#94

# 4. Generate documentation
make readme TYPE=component CATEGORY=training SUBCATEGORY=sklearn_trainer NAME=logistic_regression

# 5. Format, lint, test, and submit (same as above)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we split format + lint and test into separate steps? tests before format + lint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants