feat(mlflow): Add s3 artifact type
Context
Given the need to support other Mlflow registries other than Google Cloud Storage, this PR introduces support for S3-based registries to the artifact package, allowing other CaraML components such as Merlin and Turing to work with Mlflow that use S3-based artifact registries.
Main Changes
api/pkg/artifact/artifact.go- Created a new interfaceSchemeInterfaceand added a new implementation of theServiceinterface (S3ArtifactClient)
Other Modifications
To introduce support for S3, the AWS Golang SDK (version 2) will be introduced as a new dependency of MLP. This change requires updating the version of Go from 1.20 to 1.22, which further requires upgrades to our linter versions (which now report more errors than before), GitHub actions, etc. This PR thus includes those changes which may seem irrelevant to the original scope of this PR as described in its title.
Here's a list of some of those changes:
- Updating the version of Go to 1.22
- Updating the version of golangci-lint-action to v6 and the golangci-lint to v1.58.1
- Deleting of deprecated linters
- Naming unused arguments as
_
Random comment: If anyone finds the naming of client/service a little confusing in this artifact package, we can rename them in this PR also (I'm open for any suggestions 😅).
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 37.87%. Comparing base (
9b92012) to head (f6e5f3e).
:exclamation: There is a different number of reports uploaded between BASE (9b92012) and HEAD (f6e5f3e). Click for more details.
HEAD has 1 upload less than BASE
Flag BASE (9b92012) HEAD (f6e5f3e) api-test 2 1
Additional details and impacted files
@@ Coverage Diff @@
## main #116 +/- ##
===========================================
- Coverage 56.39% 37.87% -18.53%
===========================================
Files 48 68 +20
Lines 2477 3976 +1499
===========================================
+ Hits 1397 1506 +109
- Misses 884 2263 +1379
- Partials 196 207 +11
| Flag | Coverage Δ | |
|---|---|---|
| api-test | 37.87% <ø> (-18.53%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
It seems like the codecov report is getting a lot of unexpected coverage drops and I think it might be due to the way we are using codecov: https://docs.codecov.com/docs/unexpected-coverage-changes. I don't think I'd be able to identify and resolve the root cause in the scope of this PR though; I'll just log this as a card and then potentially remove the codecov CICD job temporarily so that it doesn't make the entire CICD pipeline appear like it failed (when in reality it's due to an unexpected codecov job failure).
Thanks for the review @tiopramayudi ! I'll be merging this PR shortly!