Feature flag refactoring (part 1) #4181

pcapriotti · 2024-08-02T09:28:16Z

First stab at refactoring feature flags and removing code duplication.

Highlights of changes:

Improved naming slightly. Features types are now called Feature, LockableFeature and LockableFeaturePatch.
TTL fields were removed from all records. They remain in the API, but are always hardcoded to be unlimited.
Turned AllFeatures into an extensible record type.
Removed WithStatusBase barbie.
Deleted obsolete computeFeatureConfigForTeamUser.
Abstracted getFeature and setFeature.
Abstracted getAllTeamFeatures.

https://wearezeta.atlassian.net/browse/WPB-10323

Checklist

Add a new entry in an appropriate subdirectory of changelog.d
Read and follow the PR guidelines

MangoIV

kind of a big one, was really nice working on it :)

libs/wire-api/src/Wire/API/Event/FeatureConfig.hs

libs/wire-api/src/Wire/API/Routes/Internal/LegalHold.hs

libs/wire-api/src/Wire/API/Team/Feature.hs

MangoIV · 2024-08-07T14:17:16Z

services/galley/src/Galley/API/Teams/Features/Get.hs

this is so nice

services/galley/src/Galley/Cassandra/FeatureTH.hs

services/galley/src/Galley/Cassandra/MakeFeature.hs

tools/stern/test/integration/API.hs

MangoIV · 2024-08-08T12:44:21Z

libs/wire-api/src/Wire/API/Event/FeatureConfig.hs

+class AllArbitraryFeatures cfgs where
+  allArbitraryFeatures :: [Gen A.Value]
+
+instance AllArbitraryFeatures '[] where
+  allArbitraryFeatures = []
+
+instance
+  ( IsFeatureConfig cfg,
+    ToSchema cfg,
+    Arbitrary cfg,
+    AllArbitraryFeatures cfgs
+  ) =>
+  AllArbitraryFeatures (cfg : cfgs)
+  where
+  allArbitraryFeatures = arbitraryFeature @cfg : allArbitraryFeatures @cfgs


I think this might be possible with recursion on SList which reifies the type level list. You also need a csontraint on all elements, so perhaps smth like cpara_SList might work this time.

services/galley/src/Galley/Cassandra/MakeFeature.hs

- rewrite boilerplate to NP combinators - regeneate nix derivations - format all - make galley, wire-subsystems and wire-api compile

Also rename FeatureSingletonMlsMigration{Config}

Co-authored-by: Mango The Fourth <[email protected]>

Also get rid of LiftForF constraint, which required `UndecidableSuperClasses`.

Also remove unnecessary parenthesis.

elland

If @MangoIV has no pending comments, LGTM.

elland · 2024-08-12T08:28:20Z

libs/wire-api/src/Wire/API/Team/Feature.hs

    dbFeatureConfig,
    dbFeatureModConfig,
-    WithStatus,


zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Aug 2, 2024

pcapriotti force-pushed the pcapriotti/feature-refactoring branch from 1253d97 to fb2ba8b Compare August 5, 2024 11:05

pcapriotti force-pushed the pcapriotti/user-features branch from 5030c22 to 0d7768d Compare August 6, 2024 12:20

pcapriotti force-pushed the pcapriotti/feature-refactoring branch 2 times, most recently from e854929 to 3c485c4 Compare August 7, 2024 09:44

MangoIV force-pushed the pcapriotti/feature-refactoring branch 2 times, most recently from b6e5ffb to f1f92c3 Compare August 7, 2024 11:54

MangoIV marked this pull request as ready for review August 7, 2024 12:16

MangoIV force-pushed the pcapriotti/feature-refactoring branch from f0d7159 to c67a907 Compare August 7, 2024 12:17

pcapriotti force-pushed the pcapriotti/feature-refactoring branch from c67a907 to 9a033f1 Compare August 7, 2024 12:19

MangoIV force-pushed the pcapriotti/user-features branch from 0d7768d to 7c5ee3c Compare August 7, 2024 12:20

MangoIV force-pushed the pcapriotti/feature-refactoring branch from 9a033f1 to a8d7ff9 Compare August 7, 2024 12:21

Base automatically changed from pcapriotti/user-features to develop August 7, 2024 13:14

pcapriotti changed the title ~~Feature flag refactoring~~ Feature flag refactoring (part 1) Aug 7, 2024

pcapriotti force-pushed the pcapriotti/feature-refactoring branch from a8d7ff9 to 550f357 Compare August 7, 2024 13:17

MangoIV reviewed Aug 7, 2024

View reviewed changes

pcapriotti force-pushed the pcapriotti/feature-refactoring branch from c23ceca to a49cdf2 Compare August 8, 2024 06:52

pcapriotti mentioned this pull request Aug 8, 2024

Feature flag refactoring (part 2) #4193

Merged

2 tasks

pcapriotti force-pushed the pcapriotti/feature-refactoring branch from 0bf9e91 to 1fa5fe1 Compare August 8, 2024 07:54

MangoIV reviewed Aug 8, 2024

View reviewed changes

pcapriotti added 10 commits August 9, 2024 09:17

Rename feature types

b87ad05

Cleanup LockableFeature

18423bd

Cleanup LockableFeaturePatch

d1b6aa5

Remove FeatureLockBase

b8822a2

Cleanup Feature

3de1a1f

Remove unused function

d6ec0b3

Replace adhoc default methods with def

6db8d66

Move MakeFeature to a separate module

bd932f4

Create StoredFeature type class

f9fa49f

Abstract setFeature

05a54ba

pcapriotti and others added 18 commits August 9, 2024 09:17

Extend MakeFeature abstraction to nullary features

3ba7688

Use Tagged to improve type inference

c503c87

Add storeFeatureLockStatus

f0f551c

Use TH to generate feature singleton cases

aa5183c

Rename unDbFeature to applyDbFeature

325b298

[feat] galley - make compile with AllFeatures represented as NP

a15c86b

- rewrite boilerplate to NP combinators - regeneate nix derivations - format all - make galley, wire-subsystems and wire-api compile

Incorporate lock status in feature queries

e00b538

Automate concatenation of rows

2bf29b7

Also rename FeatureSingletonMlsMigration{Config}

[chore] add changelog entry

00f853a

Remove todo

9d328b3

Co-authored-by: Mango The Fourth <[email protected]>

Add haddock to generateSOPInstances

70f1a1f

Co-authored-by: Mango The Fourth <[email protected]>

Recurse on all features in Arbitray Event instance

59ce0ad

Remove redundant hiding clause

1b0d45b

Remove unnecessary extensions

cae9fef

Also get rid of LiftForF constraint, which required `UndecidableSuperClasses`.

Rename mkFeature and unmkFeature

d71e2e2

Add generated code excerpt to generateTupleP

835f925

Also remove unnecessary parenthesis.

Remove unnecessary constraint

cb76183

Lint

ca74ec3

pcapriotti force-pushed the pcapriotti/feature-refactoring branch from 1fa5fe1 to ca74ec3 Compare August 9, 2024 07:24

pcapriotti added 2 commits August 9, 2024 13:44

Finish implementation of getAllFeatureConfigs

e4737a1

Fix conference calling flag assertions

e269b7e

elland approved these changes Aug 12, 2024

View reviewed changes

libs/wire-api/src/Wire/API/Team/Feature.hs

dbFeatureConfig,

dbFeatureModConfig,

WithStatus,

Copy link

Contributor

elland Aug 12, 2024

Choose a reason for hiding this comment

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

👏

pcapriotti merged commit cdeeb0b into develop Aug 12, 2024
10 checks passed

pcapriotti deleted the pcapriotti/feature-refactoring branch August 12, 2024 08:45

echoes-hq bot added echoes: technical-roadmap/technical-debt More specific category, to highlight Technical Debt being tackled. echoes: technical-roadmap/throughput More specific category, to highlight task aiming at improving the development velocity and effici... labels Aug 13, 2024

zebot mentioned this pull request Oct 9, 2024

Release 2024-10-09 - (expected chart version 5.6.0) #4288

Closed

This was referenced Oct 23, 2024

Release 2024-10-23 - (expected chart version 5.6.0) #4305

Closed

Release 2024-10-25 - (expected chart version 5.6.0) #4312

Closed

zebot mentioned this pull request Oct 30, 2024

Release 2024-10-30 - (expected chart version 5.6.0) #4321

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature flag refactoring (part 1) #4181

Feature flag refactoring (part 1) #4181

pcapriotti commented Aug 2, 2024 •

edited

Loading

MangoIV left a comment

MangoIV Aug 7, 2024

MangoIV Aug 8, 2024

elland left a comment

elland Aug 12, 2024

Feature flag refactoring (part 1) #4181

Feature flag refactoring (part 1) #4181

Conversation

pcapriotti commented Aug 2, 2024 • edited Loading

Highlights of changes:

Checklist

MangoIV left a comment

Choose a reason for hiding this comment

MangoIV Aug 7, 2024

Choose a reason for hiding this comment

MangoIV Aug 8, 2024

Choose a reason for hiding this comment

elland left a comment

Choose a reason for hiding this comment

elland Aug 12, 2024

Choose a reason for hiding this comment

pcapriotti commented Aug 2, 2024 •

edited

Loading