Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 73 additions & 39 deletions .github/workflows/first-time-contributor-pr.yml
Original file line number Diff line number Diff line change
@@ -1,51 +1,85 @@
# goal: this is meant to remind maintainers/approvers to add labels to ensure all tests are executed before merging
# and avoid merging without realizing that required tests has not been run; complementary to bulletpoint in template: https://github.com/kubeflow/model-registry/blame/de5f225d96a4daeca77506d233082b1c4ea5afa3/.github/pull_request_template.md#L21
name: Welcome first-time contributors (Beta)
name: Welcome first-time contributors (Beta2)

on:
pull_request_target:
pull_request:
types:
- opened
- synchronize
- reopened
issues:
types:
- opened

permissions: # set contents: read at top-level, per OpenSSF ScoreCard rule TokenPermissionsID
contents: read

# do NOT: add actions/checkout to this flow, add-third party scripts, or auto-trigger CI jobs
jobs:
welcome:
runs-on: ubuntu-latest
permissions:
pull-requests: write
runs-on: ubuntu-latest
issues: write
steps:
- name: Check contributor status
id: check_1st_time_contrib
uses: actions/github-script@v8
with:
script: |
const { data: pr } = await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: context.issue.number
});
const isFirstTimeContributor = pr.user.contributions === 0;
console.log(`First-time contributor status: ${isFirstTimeContributor}`);
core.setOutput('isFirstTimeContributor', isFirstTimeContributor.toString());

- name: Log contributor status (isFirstTimeContributor true)
if: steps.check_1st_time_contrib.outputs.isFirstTimeContributor == 'true'
run: echo "First-time contributor status is ${{ steps.check_1st_time_contrib.outputs.isFirstTimeContributor }}"

- name: Log contributor status (isFirstTimeContributor false)
if: steps.check_1st_time_contrib.outputs.isFirstTimeContributor == 'false'
run: echo "First-time contributor status is ${{ steps.check_1st_time_contrib.outputs.isFirstTimeContributor }}"

- name: Add a comment to the PR if first time contributor
if: steps.check_1st_time_contrib.outputs.isFirstTimeContributor == 'true'
uses: actions/github-script@v8
- name: Checkout repository
uses: actions/checkout@v4

- name: Install PyYAML
run: pip3 install pyyaml

- name: Extract approvers from OWNERS file
id: set-approvers
run: |
python3 << 'EOF'
import yaml
import os

with open('OWNERS', 'r') as f:
data = yaml.safe_load(f)

approvers = data.get('approvers', [])
result = ' '.join([f'@{approver}' for approver in approvers])

with open(os.environ['GITHUB_OUTPUT'], 'a') as f:
f.write(f'approvers={result}\n')

print(f'Extracted approvers: {result}')
EOF
- name: Log approvers
run: |
echo "Approvers: ${{ steps.set-approvers.outputs.approvers }}"

- name: Welcome first-time contributors message
uses: actions/first-interaction@v3
with:
script: |
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: "Maintainers: let's ensure the label `ok-to-test` has been maintained and all the tests have been executed before merging.<br/><br/>Thank you for your first Pull Request! 🎉🎉"
})
# do NOT: add actions/checkout to this flow, add-third party scripts, or auto-trigger CI jobs
repo_token: ${{ secrets.GITHUB_TOKEN }}
issue_message: |
🎉 **Welcome to the Kubeflow Model Registry!** 🎉
Thanks for opening your first issue! We're happy to have you as part of our community 🚀

**Here's what happens next:**
- If you'd like to contribute to this issue, check out our [Contributing Guide](https://github.com/kubeflow/model-registry/blob/main/CONTRIBUTING.md) for repo-specific guidelines and the [Kubeflow Contributor Guide](https://www.kubeflow.org/docs/about/contributing/) for general community standards
- Our team will review your issue soon!

**Join the community:**
- **Slack**: Join our [Slack channels](https://www.kubeflow.org/docs/about/community/#slack-channels)
- **Meetings**: Attend the [Kubeflow](https://www.kubeflow.org/docs/about/community/#list-of-available-meetings) online calls

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏
pr_message: |
🎉 **Welcome to the Kubeflow Model Registry!** 🎉
Thanks for opening your first PR! We're happy to have you as part of our community 🚀

**Here's what happens next:**
- If you haven't already, please check out our [Contributing Guide](https://github.com/kubeflow/model-registry/blob/main/CONTRIBUTING.md) for repo-specific guidelines and the [Kubeflow Contributor Guide](https://www.kubeflow.org/docs/about/contributing/) for general community standards
- Our team will review your PR soon!

**Join the community:**
- **Slack**: Join our [Slack channels](https://www.kubeflow.org/docs/about/community/#slack-channels)
- **Meetings**: Attend the [Kubeflow](https://www.kubeflow.org/docs/about/community/#list-of-available-meetings) online calls

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏

Note for: ${{ steps.set-approvers.outputs.approvers }}
Kindly ensure the label `ok-to-test` has been added to the PR, and all the tests have been executed before merging!
7 changes: 1 addition & 6 deletions .github/workflows/python-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,7 @@ jobs:
- name: Nox lint
working-directory: clients/python
run: |
if [[ ${{ matrix.session }} == "mypy" ]]; then
nox --python=${{ matrix.python }} ||\
echo "::error title='mypy failure'::Check the logs for more details"
else
nox --python=${{ matrix.python }}
fi
nox --python=${{ matrix.python }}

check-autogen:
name: Check autogenerated code is in sync
Expand Down
56 changes: 55 additions & 1 deletion api/openapi/catalog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ paths:
- $ref: "#/components/parameters/artifact_type"
- $ref: "#/components/parameters/artifactFilterQuery"
- $ref: "#/components/parameters/pageSize"
- $ref: "#/components/parameters/orderBy"
- $ref: "#/components/parameters/artifactOrderBy"
- $ref: "#/components/parameters/sortOrder"
- $ref: "#/components/parameters/nextPageToken"
components:
Expand Down Expand Up @@ -545,8 +545,10 @@ components:
properties:
min:
type: number
format: double
max:
type: number
format: double
FilterOptionsList:
description: List of FilterOptions
type: object
Expand Down Expand Up @@ -850,6 +852,58 @@ components:
$ref: "#/components/schemas/OrderByField"
in: query
required: false
artifactOrderBy:
style: form
explode: true
examples:
standardField:
value: ID
summary: Order by standard field
customPropertyDouble:
value: mmlu.double_value
summary: Order by custom double property
customPropertyString:
value: framework_type.string_value
summary: Order by custom string property
customPropertyInt:
value: hardware_count.int_value
summary: Order by custom integer property
name: orderBy
description: |
Specifies the order by criteria for listing artifacts.

**Standard Fields:**
- `ID` - Order by artifact ID
- `NAME` - Order by artifact name
- `CREATE_TIME` - Order by creation timestamp
- `LAST_UPDATE_TIME` - Order by last update timestamp

**Custom Property Ordering:**

Artifacts can be ordered by custom properties using the format: `<property_name>.<value_type>`

Supported value types:
- `double_value` - For numeric (floating-point) properties
- `int_value` - For integer properties
- `string_value` - For string properties

Examples:
- `mmlu.double_value` - Order by the 'mmlu' benchmark score
- `accuracy.double_value` - Order by accuracy metric
- `framework_type.string_value` - Order by framework type
- `hardware_count.int_value` - Order by hardware count
- `ttft_mean.double_value` - Order by time-to-first-token mean

**Behavior:**
- If an invalid value type is specified (e.g., `accuracy.invalid_type`), an error is returned
- If an invalid format is used (e.g., `accuracy` without `.value_type`), it falls back to ID ordering
- If a property doesn't exist, it falls back to ID ordering
- Artifacts with the specified property are ordered first (by the property value), followed by artifacts without the property (ordered by ID)
- Empty property names (e.g., `.double_value`) return an error
schema:
type: string
in: query
required: false
labelOrderBy:
style: form
explode: true
Expand Down
56 changes: 55 additions & 1 deletion api/openapi/src/catalog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ paths:
- $ref: "#/components/parameters/artifact_type"
- $ref: "#/components/parameters/artifactFilterQuery"
- $ref: "#/components/parameters/pageSize"
- $ref: "#/components/parameters/orderBy"
- $ref: "#/components/parameters/artifactOrderBy"
- $ref: "#/components/parameters/sortOrder"
- $ref: "#/components/parameters/nextPageToken"
components:
Expand Down Expand Up @@ -415,8 +415,10 @@ components:
properties:
min:
type: number
format: double
max:
type: number
format: double
FilterOptionsList:
description: List of FilterOptions
type: object
Expand Down Expand Up @@ -571,6 +573,58 @@ components:
$ref: "#/components/schemas/OrderByField"
in: query
required: false
artifactOrderBy:
style: form
explode: true
examples:
standardField:
value: ID
summary: Order by standard field
customPropertyDouble:
value: mmlu.double_value
summary: Order by custom double property
customPropertyString:
value: framework_type.string_value
summary: Order by custom string property
customPropertyInt:
value: hardware_count.int_value
summary: Order by custom integer property
name: orderBy
description: |
Specifies the order by criteria for listing artifacts.

**Standard Fields:**
- `ID` - Order by artifact ID
- `NAME` - Order by artifact name
- `CREATE_TIME` - Order by creation timestamp
- `LAST_UPDATE_TIME` - Order by last update timestamp

**Custom Property Ordering:**

Artifacts can be ordered by custom properties using the format: `<property_name>.<value_type>`

Supported value types:
- `double_value` - For numeric (floating-point) properties
- `int_value` - For integer properties
- `string_value` - For string properties

Examples:
- `mmlu.double_value` - Order by the 'mmlu' benchmark score
- `accuracy.double_value` - Order by accuracy metric
- `framework_type.string_value` - Order by framework type
- `hardware_count.int_value` - Order by hardware count
- `ttft_mean.double_value` - Order by time-to-first-token mean

**Behavior:**
- If an invalid value type is specified (e.g., `accuracy.invalid_type`), an error is returned
- If an invalid format is used (e.g., `accuracy` without `.value_type`), it falls back to ID ordering
- If a property doesn't exist, it falls back to ID ordering
- Artifacts with the specified property are ordered first (by the property value), followed by artifacts without the property (ordered by ID)
- Empty property names (e.g., `.double_value`) return an error
schema:
type: string
in: query
required: false
labelOrderBy:
style: form
explode: true
Expand Down
8 changes: 8 additions & 0 deletions catalog/cmd/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net/http"
"reflect"
"time"

"github.com/golang/glog"
"github.com/kubeflow/model-registry/catalog/internal/catalog"
Expand Down Expand Up @@ -62,6 +63,7 @@ func runCatalogServer(cmd *cobra.Command, args []string) error {
getRepo[models.CatalogArtifactRepository](repoSet),
getRepo[models.CatalogModelArtifactRepository](repoSet),
getRepo[models.CatalogMetricsArtifactRepository](repoSet),
getRepo[models.PropertyOptionsRepository](repoSet),
)

loader := catalog.NewLoader(services, catalogCfg.ConfigPath)
Expand All @@ -72,6 +74,12 @@ func runCatalogServer(cmd *cobra.Command, args []string) error {
}
loader.RegisterEventHandler(perfLoader.Load)

poRefresher := models.NewPropertyOptionsRefresher(context.Background(), services.PropertyOptionsRepository, time.Second)
loader.RegisterEventHandler(func(ctx context.Context, record catalog.ModelProviderRecord) error {
poRefresher.Trigger()
return nil
})

err = loader.Start(context.Background())
if err != nil {
return fmt.Errorf("error loading catalog sources: %v", err)
Expand Down
2 changes: 1 addition & 1 deletion catalog/internal/catalog/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type ListModelsParams struct {
type ListArtifactsParams struct {
FilterQuery string
PageSize int32
OrderBy model.OrderByField
OrderBy string
SortOrder model.SortOrder
NextPageToken *string
ArtifactTypesFilter []string
Expand Down
Loading