Decouple MLMD types setup from Go core library

[tarilabs]

resolves #265

Description

How Has This Been Tested?

Merge criteria:

• The commits and have meaningful messages; the author will squash them after approval or will ask to merge with squash.
• Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• The developer has manually tested the changes and verified that the changes work

[lampajr]

LGTM, thanks a lot @tarilabs !

[lampajr]

Just for the records, in a followup PR we could move some tests from core_test.go to a dedicated mlmdtypes_test.go as they are actually testing the MLMD types setup, e.g., these ones https://github.com/opendatahub-io/model-registry/blob/2cebe8f6d2f0d876b9e80f4d506277bb99a1c54f/pkg/core/core_test.go#L244-L533 where the main purpose is to ensure the types are correctly setup.

I would suggest to move it in a separate PR (if we agree on the change itself obv) as this is not a urgent topic right now and it could require some refactoring in the testcontainers setup.

[tarilabs]

...or we could simply rename core_test.go into integration_test.go since it's what's actually is 🙃

[lampajr]

...or we could simply rename core_test.go into integration_test.go since it's what's actually is 🙃

fair enough, valid alternative 😄

[kamuridesu]

Hello! Could you please answer a question for me? Why use pointers for constants, as in https://github.com/opendatahub-io/model-registry/pull/267/files#diff-04387a737874b37f9edc50aec64cd72c86d4e5e876deae7bee3463a2681ac79cR14?
I'm trying to study the code for learning

[tarilabs]

Hello! Could you please answer a question for me? Why use pointers for constants, as in https://github.com/opendatahub-io/model-registry/pull/267/files#diff-04387a737874b37f9edc50aec64cd72c86d4e5e876deae7bee3463a2681ac79cR14? I'm trying to study the code for learning

Hi @kamuridesu it's more about the receivers of those pointers, the receivers need a pointer (to a string, to a boolean, etc).

On other hand, the pointer of what we need, can be built from a constant; so instead of redefining a brand new pointer each time, we just simplify at the top of the file.

Please notice our need is for those pointer to point to a value which semantically is a constant for us (humans). In Go there is no support afaik of getting address of constants, so we need to assign the constant value to a variable, and take that variable address. Instead of doing that over and over, we just "condensed" this way.

Hope this clarifies?

[kamuridesu]

Thank you

[codecov-commenter]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (0497cf4) 74.16% compared to head (5f65042) 72.57%.

Files	Patch %	Lines
pkg/core/core.go	76.59%	8 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #267      +/-   ##
==========================================
- Coverage   74.16%   72.57%   -1.59%     
==========================================
  Files          17       17              
  Lines        2295     2239      -56     
  Branches       73       73              
==========================================
- Hits         1702     1625      -77     
- Misses        426      440      +14     
- Partials      167      174       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lampajr]

The failure is unrelated to the change as it fails downloading the openapi-generator-cli:

#20 [builder 14/14] RUN CGO_ENABLED=1 GOOS=linux GOARCH=amd64 make clean model-registry
#20 0.195 rm -Rf ./model-registry internal/ml_metadata/proto/*.go internal/converter/generated/*.go pkg/openapi
#20 0.202 protoc -I./api/grpc --go_out=./internal --go_opt=paths=source_relative \
#20 0.202 	--go-grpc_out=./internal --go-grpc_opt=paths=source_relative api/grpc/ml_metadata/proto/metadata_store.proto
#20 0.282 protoc -I./api/grpc --go_out=./internal --go_opt=paths=source_relative \
#20 0.282 	--go-grpc_out=./internal --go-grpc_opt=paths=source_relative api/grpc/ml_metadata/proto/metadata_store_service.proto
#20 0.427 openapi-generator-cli validate -i api/openapi/model-registry.yaml
#20 0.496 /workspace/bin/openapi-generator-installation/lib/node_modules/@openapitools/openapi-generator-cli/node_modules/@nestjs/common/file-stream/streamable-file.js:36
#20 0.496             this.options.length ??= bufferOrReadStream.length;
#20 0.496                                 ^^^
#20 0.496 
#20 0.496 SyntaxError: Unexpected token '??='
#20 0.496     at wrapSafe (internal/modules/cjs/loader.js:1029:16)
#20 0.496     at Module._compile (internal/modules/cjs/loader.js:1078:27)
#20 0.496     at Object.Module._extensions..js (internal/modules/cjs/loader.js:1143:10)
#20 0.496     at Module.load (internal/modules/cjs/loader.js:979:32)
#20 0.496     at Function.Module._load (internal/modules/cjs/loader.js:819:12)
#20 0.496     at Module.require (internal/modules/cjs/loader.js:1003:19)
#20 0.496     at require (internal/modules/cjs/helpers.js:107:18)
#20 0.496     at Object.<anonymous> (/workspace/bin/openapi-generator-installation/lib/node_modules/@openapitools/openapi-generator-cli/node_modules/@nestjs/common/file-stream/index.js:4:22)
#20 0.496     at Module._compile (internal/modules/cjs/loader.js:1114:14)
#20 0.496     at Object.Module._extensions..js (internal/modules/cjs/loader.js:1143:10)
#20 0.499 make: *** [Makefile:70: openapi/validate] Error 1
#20 ERROR: process "/bin/sh -c CGO_ENABLED=1 GOOS=linux GOARCH=amd64 make clean model-registry" did not complete successfully: exit code: 2

[tarilabs]

The failure is unrelated to the change as it fails downloading the openapi-generator-cli

yep, also taking note in: https://github.com/opendatahub-io/model-registry/pull/270#pullrequestreview-1838383031

[tarilabs]

This has been open for 9 days now and I didn't notice any substantial request for changes. All green. Merging and if any change required can be dealt with in subsequent PRs I guess.