Skip to content

feat: add mediaType to the blob and location#573

Closed
Skarlso wants to merge 3 commits into
open-component-model:mainfrom
Skarlso:add-media-type-to-location
Closed

feat: add mediaType to the blob and location#573
Skarlso wants to merge 3 commits into
open-component-model:mainfrom
Skarlso:add-media-type-to-location

Conversation

@Skarlso

@Skarlso Skarlso commented Aug 15, 2025

Copy link
Copy Markdown
Contributor

What this PR does / why we need it

I know, I know. I will release the modules separately, but instead of a gazillion prs and discussions on separate pages, I wanted to discuss the changes together because they are small enough.

Fixed #564

Part Of open-component-model/ocm-project#553 and needed for #530

Which issue(s) this PR fixes

@Skarlso Skarlso requested a review from a team as a code owner August 15, 2025 11:59
@github-actions github-actions Bot added kind/feature new feature, enhancement, improvement, extension size/m Medium labels Aug 15, 2025
Comment thread bindings/go/helm/cmd/main.go Outdated
Signed-off-by: Gergely Brautigam <gergely.brautigam@sap.com>
@Skarlso Skarlso force-pushed the add-media-type-to-location branch from dcb8535 to e85abf2 Compare August 15, 2025 12:02
Comment thread bindings/go/blob/filesystem/file_blob.go Outdated
Comment thread bindings/go/blob/filesystem/file_blob.go Outdated
Comment thread bindings/go/blob/filesystem/file_blob.go Outdated
Comment thread bindings/go/helm/go.mod
)

replace (
ocm.software/open-component-model/bindings/go/blob => ../blob

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think these will disappear right?

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.

absolutely. I'll have separate prs ready once the whole looks okay. :)

Skarlso and others added 2 commits August 19, 2025 13:10
…nd default file mediatype

On-behalf-of: gergely.brautigam@sap.com

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso Skarlso force-pushed the add-media-type-to-location branch from 02232f0 to f6ae8d2 Compare August 19, 2025 11:13
return nil, fmt.Errorf("error copying blob data: %w", err)
}

mediaType := "application/x-gzip"

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.

Maybe I'll just leave it as an empty string?

Comment on lines +22 to +33

if err != nil {
return nil, err
}

if location.MediaType != "" {
if mtOverrideable, ok := b.(blob.MediaTypeOverrideable); ok {
mtOverrideable.SetMediaType(location.MediaType)
}
}

return b, nil

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.

This will be replaced with

// CreateBlobData creates a blob based on the location.
func CreateBlobData(location types.Location) (blob.ReadOnlyBlob, error) {
	var b blob.Blob
	var err error

	switch location.LocationType {
	case types.LocationTypeLocalFile:
		b, err = filesystem.GetBlobFromOSPath(location.Value)
	default:
		return nil, fmt.Errorf("unsupported location type: %s", location.LocationType)
	}
	if err != nil {
		return nil, err
	}

	w := direct.New(direct.NewReadCloserWrapper(b), direct.WithMediaType(location.MediaType))
	return w, nil
}

Once #593 is merged.

jakobmoellerdev pushed a commit that referenced this pull request Aug 21, 2025
…#593)

<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it

This creates a wrapper for ReadOnlyBlobs so they can act like a
MediaType aware blob.

This is part of
#573.

The reason here is, that we don't want filesystem to be modified to be
media type aware. Rather we can wrap it in a direct blob that will be
able to call the underlying reader and even add capabilities to it, like
being media type aware.

Such a wrapper could be created like this:
```go
w := direct.New(direct.NewReadCloserWrapper(b), direct.WithMediaType(location.MediaType))
```

This will create a wrapper that is a `blob.ReadOnlyBlob`.

#### Which issue(s) this PR fixes
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->

---------

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Skarlso added a commit that referenced this pull request Aug 22, 2025
<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it

This is the plugin part for
#573.

Another will be the implementation on the plugin once mediatype is made
available.

#### Which issue(s) this PR fixes
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Co-authored-by: Jakob Möller <jakob.moeller@sap.com>
jakobmoellerdev pushed a commit that referenced this pull request Aug 22, 2025
<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it

Part of
#573
that modifies helm plugin.

#### Which issue(s) this PR fixes
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso

Skarlso commented Aug 22, 2025

Copy link
Copy Markdown
Contributor Author

This is now done as all interim parts have been merged. 🎉

@Skarlso Skarlso closed this Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature new feature, enhancement, improvement, extension size/m Medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Blob and location needs to be media type aware

3 participants