Skip to content

Delete vParquet2 format#6071

Merged
mdisibio merged 3 commits intografana:mainfrom
mdisibio:delete-vparquet2-2
Dec 12, 2025
Merged

Delete vParquet2 format#6071
mdisibio merged 3 commits intografana:mainfrom
mdisibio:delete-vparquet2-2

Conversation

@mdisibio
Copy link
Copy Markdown
Contributor

@mdisibio mdisibio commented Dec 11, 2025

What this PR does:
Following #5688 and #6062 , we can now remove vParquet2 for the next release.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

/*if b.Version == "vParquet2" {
return fmt.Errorf(DeprecatedError, "vParquet2", "vParquet3")
}
}*/
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.

Thoughts on removing vs commenting out?

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.

I left that there because tracking down that logic check next time we deprecate a version. The issue is that the location of this check can't reference anything in encoding/ or encoding/vParquetX because then it would create a cyclic dependency. Is there a different way we could comment this to be clearer?

Comment thread tempodb/config_test.go
cfg: &Config{
WAL: &wal.Config{
Version: vparquet2.VersionString,
Version: "vParquet5-preview1", // Turned into unsupported
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.

Is this a different change/related?

Copy link
Copy Markdown
Contributor Author

@mdisibio mdisibio Dec 11, 2025

Choose a reason for hiding this comment

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

We are deleting the vparquet2 module so we cannot use that string from it anymore. All we need here is a version that is parseable/recognized, but doesn't support writes. Since there isn't another deprecated version, that is the next best thing.

@javiermolinar javiermolinar added this to the v2.10 milestone Dec 12, 2025
Copy link
Copy Markdown
Contributor

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

Lgtm

@mdisibio mdisibio merged commit ac4d4b6 into grafana:main Dec 12, 2025
37 of 38 checks passed
oleg-kozlyuk-grafana pushed a commit to oleg-kozlyuk-grafana/tempo that referenced this pull request Dec 13, 2025
* Delete vParquet2

* changelog

* Update tempo cli docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants