Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[schema processor] Part 3 - Modifiers and Revisions #12147 #17020

Merged
merged 15 commits into from
May 23, 2023
Merged

[schema processor] Part 3 - Modifiers and Revisions #12147 #17020

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
58a7133
[schema processor] Modifier
MovieStoreGuy Jul 7, 2022
49ba868
Adding changes
MovieStoreGuy Feb 24, 2023
af63a00
Adding migrate package.
MovieStoreGuy May 7, 2023
6dd9692
Removing modiferies to avoid confusion
MovieStoreGuy May 8, 2023
e5298f3
Fixing up naming issues
MovieStoreGuy May 8, 2023
bf084ac
Adding checks in for building a revision
MovieStoreGuy May 8, 2023
4d3ead6
Fixing up replaces
MovieStoreGuy May 9, 2023
78d6abf
Removing transform files
MovieStoreGuy May 22, 2023
084bc74
Fixing naming
MovieStoreGuy May 10, 2023
df76e55
Fixing up conflict
MovieStoreGuy May 10, 2023
811463d
Fixing typo
MovieStoreGuy May 10, 2023
b913d18
Updating naming
MovieStoreGuy May 11, 2023
285982f
Fixing up ast usage
MovieStoreGuy May 17, 2023
83c76a2
FIxing up generated mod files
MovieStoreGuy May 22, 2023
69cf2bb
Fixing up license headers
MovieStoreGuy May 23, 2023
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
Prev Previous commit
Next Next commit
Updating naming
  • Loading branch information
@MovieStoreGuy
MovieStoreGuy committed May 22, 2023
commit b913d185e5ed19a72e77523028bf67c0f6e98809
4 changes: 2 additions & 2 deletions processor/schemaprocessor/internal/migrate/attributes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ type AttributeChangeSet struct {
// and be applied sequentially to ensure deterministic behavior.
type AttributeChangeSetSlice []*AttributeChangeSet
tigrannajaryan marked this conversation as resolved.
Show resolved Hide resolved

// NewAttributes allows for typed strings to be used as part
// NewAttributeChangeSet allows for typed strings to be used as part
// of the invocation that will be converted into the default string type.
func NewAttributes[Key AttributeKey, Value AttributeKey](mappings map[Key]Value) *AttributeChangeSet {
func NewAttributeChangeSet[Key AttributeKey, Value AttributeKey](mappings map[Key]Value) *AttributeChangeSet {
attr := &AttributeChangeSet{
updates: make(map[string]string, len(mappings)),
rollback: make(map[string]string, len(mappings)),
Expand Down
28 changes: 14 additions & 14 deletions processor/schemaprocessor/internal/migrate/attributes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestNewAttributeChangeSet(t *testing.T) {
t.Run("map of strings", func(t *testing.T) {
t.Parallel()

acs := NewAttributes(map[string]string{
acs := NewAttributeChangeSet(map[string]string{
"hello": "world",
})

Expand All @@ -57,7 +57,7 @@ func TestNewAttributeChangeSet(t *testing.T) {
t.Run("typedef string values", func(t *testing.T) {
t.Parallel()

acs := NewAttributes(map[testTypeDefKey]testTypeDefValue{
acs := NewAttributeChangeSet(map[testTypeDefKey]testTypeDefValue{
"awesome": "awesome.service",
})

Expand Down Expand Up @@ -86,7 +86,7 @@ func TestAttributeChangeSetApply(t *testing.T) {
}{
{
name: "no modifications",
acs: NewAttributes(map[string]string{}),
acs: NewAttributeChangeSet(map[string]string{}),
attrs: testHelperBuildMap(func(m pcommon.Map) {
m.PutInt("test.cases", 1)
}),
Expand All @@ -96,7 +96,7 @@ func TestAttributeChangeSetApply(t *testing.T) {
},
{
name: "Apply changes",
acs: NewAttributes(map[string]string{
acs: NewAttributeChangeSet(map[string]string{
"service_version": "service.version",
}),
attrs: testHelperBuildMap(func(m pcommon.Map) {
Expand All @@ -110,7 +110,7 @@ func TestAttributeChangeSetApply(t *testing.T) {
// it forces both values to be removed from the the attributes.
{
name: "naming loop",
acs: NewAttributes(map[string]string{
acs: NewAttributeChangeSet(map[string]string{
"service.version": "service_version",
"service_version": "service.version",
}),
Expand All @@ -123,7 +123,7 @@ func TestAttributeChangeSetApply(t *testing.T) {
},
{
name: "overrides existing value",
acs: NewAttributes(map[string]string{
acs: NewAttributeChangeSet(map[string]string{
"application.name": "service.name",
}),
attrs: testHelperBuildMap(func(m pcommon.Map) {
Expand Down Expand Up @@ -163,7 +163,7 @@ func TestAttributeChangeSetRollback(t *testing.T) {
}{
{
name: "no modifications",
acs: NewAttributes(map[string]string{}),
acs: NewAttributeChangeSet(map[string]string{}),
attrs: testHelperBuildMap(func(m pcommon.Map) {
m.PutInt("test.cases", 1)
}),
Expand All @@ -173,7 +173,7 @@ func TestAttributeChangeSetRollback(t *testing.T) {
},
{
name: "Apply changes",
acs: NewAttributes(map[string]string{
acs: NewAttributeChangeSet(map[string]string{
"service_version": "service.version",
}),
attrs: testHelperBuildMap(func(m pcommon.Map) {
Expand All @@ -185,7 +185,7 @@ func TestAttributeChangeSetRollback(t *testing.T) {
},
{
name: "naming loop",
acs: NewAttributes(map[string]string{
acs: NewAttributeChangeSet(map[string]string{
"service.version": "service_version",
"service_version": "service.version",
}),
Expand All @@ -198,7 +198,7 @@ func TestAttributeChangeSetRollback(t *testing.T) {
},
{
name: "overrides existing value",
acs: NewAttributes(map[string]string{
acs: NewAttributeChangeSet(map[string]string{
"application.name": "service.name",
}),
attrs: testHelperBuildMap(func(m pcommon.Map) {
Expand Down Expand Up @@ -248,10 +248,10 @@ func TestNewAttributeChangeSetSliceApply(t *testing.T) {
{
name: "changes defined",
changes: NewAttributeChangeSetSlice(
NewAttributes(map[string]string{
NewAttributeChangeSet(map[string]string{
"service_version": "service.version",
}),
NewAttributes(map[string]string{
NewAttributeChangeSet(map[string]string{
"service.version": "application.service.version",
}),
),
Expand Down Expand Up @@ -295,10 +295,10 @@ func TestNewAttributeChangeSetSliceApplyRollback(t *testing.T) {
{
name: "changes defined",
changes: NewAttributeChangeSetSlice(
NewAttributes(map[string]string{
NewAttributeChangeSet(map[string]string{
"service_version": "service.version",
}),
NewAttributes(map[string]string{
NewAttributeChangeSet(map[string]string{
"service.version": "application.service.version",
}),
),
Expand Down
2 changes: 1 addition & 1 deletion processor/schemaprocessor/internal/migrate/conditional.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func NewConditionalAttributeSet[Key AttributeKey, Value AttributeKey, Match Valu
}
return &ConditionalAttributeSet{
on: &on,
attrs: NewAttributes(mappings),
attrs: NewAttributeChangeSet(mappings),
}
}

Expand Down
2 changes: 1 addition & 1 deletion processor/schemaprocessor/internal/translation/revision_v1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func newAttributeChangeSetSliceFromChanges(attrs ast.Attributes) *migrate.Attrib
values := make([]*migrate.AttributeChangeSet, 0, 10)
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason to use 10 as capacity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was just a random number I pulled from no where.

Copy link
Member

Choose a reason for hiding this comment

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

Nit: since it has no particular importance I think it can be safely deleted, values := make([]*migrate.AttributeChangeSet) should be good enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, (not sure when), but the latest version of go requires at least len and an optional cap.

Tested it here (to make sure it wasn't a linter getting it in the way)
https://go.dev/play/p/9uNpeSYw6gN

Copy link
Member

Choose a reason for hiding this comment

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

You are right. Don't know what I was thinking. What I probably wanted to say is that you can start with an empty slice since append later will happily allocate as many as needed:

var values []*migrate.AttributeChangeSet

for _, at := range attrs.Changes {
if renamed := at.RenameAttributes; renamed != nil {
values = append(values, migrate.NewAttributes(renamed.AttributeMap))
values = append(values, migrate.NewAttributeChangeSet(renamed.AttributeMap))
}
}
return migrate.NewAttributeChangeSetSlice(values...)
Expand Down
6 changes: 3 additions & 3 deletions processor/schemaprocessor/internal/translation/revision_v1_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,15 @@ func TestNewRevision(t *testing.T) {
expect: &RevisionV1{
ver: &Version{1, 0, 0},
all: migrate.NewAttributeChangeSetSlice(
migrate.NewAttributes(map[string]string{
migrate.NewAttributeChangeSet(map[string]string{
"state": "status",
}),
migrate.NewAttributes(map[string]string{
migrate.NewAttributeChangeSet(map[string]string{
"status": "state",
}),
),
resource: migrate.NewAttributeChangeSetSlice(
migrate.NewAttributes(map[string]string{
migrate.NewAttributeChangeSet(map[string]string{
"service_name": "service.name",
}),
),
Expand Down