Page MenuHomePhabricator

[services][feature-flags] Introduce map and i32 parsing
ClosedPublic

Authored by tomek on Feb 23 2023, 7:27 AM.
Tags
None
Referenced Files
F3394379: D6855.id23139.diff
Sat, Nov 30, 7:24 PM
F3394203: D6855.diff
Sat, Nov 30, 6:14 PM
Unknown Object (File)
Thu, Nov 28, 2:36 AM
Unknown Object (File)
Wed, Nov 13, 1:42 AM
Unknown Object (File)
Oct 26 2024, 10:18 PM
Unknown Object (File)
Oct 26 2024, 10:18 PM
Unknown Object (File)
Oct 26 2024, 10:17 PM
Unknown Object (File)
Oct 26 2024, 10:17 PM
Subscribers

Details

Summary

We have to introduce a function that parses strings into numbers, because map keys in Dynamo are plain strings instead of AttributeValues. This affects what we store in DBItemError - now it is either AttributeValue or a String.

Depends on D6854

Test Plan

Tested with the rest of the stack.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

services/feature-flags/src/database.rs
40–44 ↗(On Diff #23005)

This is copied from other services. I had to remove it from the previous diff as this case couldn't be matched.

tomek requested review of this revision.Feb 23 2023, 7:43 AM
bartek requested changes to this revision.Feb 24 2023, 1:41 AM
bartek added inline comments.
services/feature-flags/src/database.rs
101–109 ↗(On Diff #23005)

Don't you want to distinguish between IncorrectType and Missing, the same way as in parse_{string,bool}_attribute?

If this is done deliberately, just let me know

116–123 ↗(On Diff #23005)

Wouldn't it work this way? Or are error types incompatible?

This revision now requires changes to proceed.Feb 24 2023, 1:41 AM
services/feature-flags/src/database.rs
101–109 ↗(On Diff #23005)

That makes sense - changed

116–123 ↗(On Diff #23005)

It should work. I updated the diff and will test it later, after I'm done with rebasing.

Reabase and make error handling more consistent with other places.

This revision is now accepted and ready to land.Feb 27 2023, 5:02 AM