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.

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

feat: type-safe conversion for line-protocol Addfield #43 #42

Closed

Conversation

arukiidou
Copy link

@arukiidou arukiidou commented Sep 25, 2023

Proposed Changes

  • Add line-protocol compliant wrapper
  • Add generic type constraints
    • line-protocol type
      • Float
      • Integer
      • Uinteger
      • String
      • Bool
    • native go <> line-protocol shortcut
    • time.Time conversion
    • time.Duration - using fmt.Stringer
func (m *Point) AddFieldFromValue(k string, v lineprotocol.Value) *Point {}
func NewValueFromInt[I Integer](v I) lineprotocol.Value {}

Usage

func example() {
	p := NewPoint("measurement", map[string]string{}, map[string]interface{}{"measurement": "air"}, time.Now())
        // unsupported type.
	p.AddFieldFromValue("✅ Compile error.", cmplx.Inf())
        // supported type.
	p.AddFieldFromValue("✅ This is safe.", NewValueFromInt(255))
}

Checklist

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • Tests pass
  • Commit messages are conventional
  • Sign CLA (if not already signed)

@bednar
Copy link
Member

bednar commented Sep 26, 2023

@arukiidou, thank you for using our client and for your PR. We're excited to review it once it's ready.

@arukiidou arukiidou force-pushed the feature/add-guardrail branch 3 times, most recently from 10fcc48 to 377043a Compare October 1, 2023 14:48
@arukiidou arukiidou marked this pull request as ready for review October 1, 2023 14:49
@arukiidou arukiidou changed the title [Draft] Support Type Guard feat: type-safe conversion for line-protocol Addfield #43 Oct 1, 2023
@arukiidou
Copy link
Author

ready for review. 👍
rebased behind of v0.3.0

@arukiidou
Copy link
Author

fixed,sorry🙏

float64 | int64 | uint64 | string | []byte | bool
}

// [Float] is IEEE-754 64-bit floating-point numbers. Default numerical type. InfluxDB supports scientific notation in float field values.
Copy link
Member

Choose a reason for hiding this comment

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

Add Float as a prefix to docs:

Suggested change
// [Float] is IEEE-754 64-bit floating-point numbers. Default numerical type. InfluxDB supports scientific notation in float field values.
// Float [Float] is IEEE-754 64-bit floating-point numbers. Default numerical type. InfluxDB supports scientific notation in float field values.

Copy link
Author

Choose a reason for hiding this comment

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

fixed.

~float32 | ~float64
}

// [Integer] is signed 64-bit integers.
Copy link
Member

Choose a reason for hiding this comment

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

Add Integer as a prefix to docs:

Suggested change
// [Integer] is signed 64-bit integers.
// Integer [Integer] is signed 64-bit integers.

Copy link
Author

Choose a reason for hiding this comment

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

fixed.

~int | ~int8 | ~int16 | ~int32 | ~int64
}

// [UInteger] is unsigned 64-bit integers.
Copy link
Member

Choose a reason for hiding this comment

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

Add UInteger as a prefix to docs:

Suggested change
// [UInteger] is unsigned 64-bit integers.
// UInteger [UInteger] is unsigned 64-bit integers.

Copy link
Author

Choose a reason for hiding this comment

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

fixed.

~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64
}

// [String] is plain text string. Length limit 64KB.
Copy link
Member

Choose a reason for hiding this comment

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

Add String as a prefix to docs:

Suggested change
// [String] is plain text string. Length limit 64KB.
// String [String] is plain text string. Length limit 64KB.

Copy link
Author

Choose a reason for hiding this comment

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

fixed.

~string | ~[]byte
}

// [Boolean] is true or false values.
Copy link
Member

Choose a reason for hiding this comment

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

Add Boolean as a prefix to docs:

Suggested change
// [Boolean] is true or false values.
// Boolean [Boolean] is true or false values.

Copy link
Author

Choose a reason for hiding this comment

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

fixed.

@bednar
Copy link
Member

bednar commented Oct 3, 2023

I have a question regarding the use of lineprotocol.Value. Is its usage necessary? What do you think about adding something like AddStringField(k string, v string) to Point?

func (m *Point) AddStringField(k string, v string) *Point {
	m.AddField(k, v)
	return m
}

@arukiidou
Copy link
Author

arukiidou commented Oct 4, 2023

I have a question regarding the use of lineprotocol.Value. Is its usage necessary? What do you think about adding something like AddStringField(k string, v string) to Point?

func (m *Point) AddStringField(k string, v string) *Point {
	m.AddField(k, v)
	return m
}
  • Oh, I prefer AddStringField better.

@arukiidou arukiidou mentioned this pull request Oct 4, 2023
@bednar
Copy link
Member

bednar commented Oct 6, 2023

We aim to synchronize the API across our v3 clients — js, go, java, and c#. We've already updated the API in both the c# and js clients. For more information, see:

JS Client PR #89
JS Client PR #89 File Diff
C# Client PR #52
C# Client PR #52 File Diff

@arukiidou arukiidou force-pushed the feature/add-guardrail branch 2 times, most recently from 5f4b40c to d741d07 Compare October 18, 2023 10:25
@arukiidou arukiidou requested a review from bednar October 18, 2023 10:27
@arukiidou arukiidou marked this pull request as draft October 18, 2023 10:28
@bednar
Copy link
Member

bednar commented Oct 18, 2023

@arukiidou thanks again for your PR. Can you please align your code with upcoming changes for structured query - #45?

@arukiidou
Copy link
Author

@arukiidou thanks again for your PR. Can you please align your code with upcoming changes for structured query - #45?

Yes, I will do so. It may take some time.

Signed-off-by: junya koyama <arukiidou@yahoo.co.jp>
Signed-off-by: junya koyama <arukiidou@yahoo.co.jp>
Signed-off-by: junya koyama <arukiidou@yahoo.co.jp>
Signed-off-by: junya koyama <arukiidou@yahoo.co.jp>
Signed-off-by: junya koyama <arukiidou@yahoo.co.jp>
@arukiidou
Copy link
Author

@bednar

@arukiidou arukiidou closed this Oct 29, 2023
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.

Type-safe conversion for line-protocol Addfield
2 participants