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

[flagd/in-process] float flags with 0.0 value are parsed as ints #470

Closed
toddbaert opened this issue Oct 2, 2023 · 5 comments · Fixed by #472
Closed

[flagd/in-process] float flags with 0.0 value are parsed as ints #470

toddbaert opened this issue Oct 2, 2023 · 5 comments · Fixed by #472
Assignees

Comments

@toddbaert
Copy link
Member

toddbaert commented Oct 2, 2023

The flagd in-process provider should be able to resolve ints as floats, and vice-versa.

To fix:

  • modify this area of the code to support conversion from int/float, float/int
  • add relevant tests here
  • un-comment float tests here

Background:

Originally posted by @toddbaert in #468 (comment)

@Kavindu-Dodan
Copy link
Collaborator

I summarised my findings here - https://github.com/open-feature/java-sdk-contrib/pull/468/files#r1344520202

IMO this can be safely fixed at the in-process provider end. A special handling for Double should fix this

@toddbaert
Copy link
Member Author

IMO this can be safely fixed at the in-process provider end. A special handling for Double should fix this

I was thinking this too... in fact, to be consistent with RPC, I think we have to attempt conversion in both directions, int -> float and float -> int (RPC does this). Additionally, if we do this, we may need to specify how the float -> int conversion is done... I think Java and Go both round toward zero... I believe .NET does too. This might be worth specifying.

I will proceed with a fix as described above unless @Kavindu-Dodan or @beeme1mr disagrees.

@beeme1mr
Copy link
Member

beeme1mr commented Oct 4, 2023

Yes, I agree that we should cast the values. This should be documented in the spec.

@DBlanchard88
Copy link
Contributor

I think i understand how to do this. Ill take a shot at it.

@toddbaert
Copy link
Member Author

I think i understand how to do this. Ill take a shot at it.

Alright, cool. Assigned.

DBlanchard88 pushed a commit to DBlanchard88/java-sdk-contrib that referenced this issue Apr 29, 2024
…pen-feature#470)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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 a pull request may close this issue.

4 participants