Added TTL query field to features API. by elland · Pull Request #2417 · wireapp/wire-server

elland · 2022-05-19T11:10:28Z

Checklist

The PR Title explains the impact of the change.
The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
changelog.d contains the following bits of information (details):
- A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

elland

Left some open questions.

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

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

battermann

Looking good
An integration would be nice
Does it make sense to use days for the TTL in stern and seconds in the galley API? In this case we can have an integration test with a very short TTL.

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

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

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

fisx

looks good so far!

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

nix/default.nix

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

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

services/galley/src/Galley/API/Internal.hs

fisx · 2022-05-31T08:54:49Z

is there anything else missing besides an approval, btw? if not could you complete the check list in the description?

fisx

please sync with the people working on #2435 before merging, i'm sure there will be lots of ugly conflicts...

changelog.d/1-api-changes/added-ttl-param-to-back-office-feature-api

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

libs/wire-api/src/Wire/API/Routes/Public/Galley.hs

tools/stern/src/Stern/API.hs

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

services/galley/test/integration/API/Teams/Feature.hs

Sad: stern is not tested, so we did not catch this with a test.

fisx

Found a few more details. Please take a look at my changes and comments. No further review needed if you're happy.

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

tools/stern/src/Stern/Intra.hs

elland temporarily deployed to cachix May 19, 2022 11:10 Inactive

elland force-pushed the add-ttl-to-feature-api branch from 04d90e0 to 6893f8c Compare May 19, 2022 11:15

elland temporarily deployed to cachix May 19, 2022 11:15 Inactive

elland commented May 19, 2022

View reviewed changes

libs/wire-api/src/Wire/API/Team/Feature.hs Outdated Show resolved Hide resolved

libs/wire-api/src/Wire/API/Team/Feature.hs Outdated Show resolved Hide resolved

services/galley/src/Galley/Cassandra/TeamFeatures.hs Outdated Show resolved Hide resolved

battermann reviewed May 19, 2022

View reviewed changes

services/galley/src/Galley/Cassandra/TeamFeatures.hs Outdated Show resolved Hide resolved

services/galley/src/Galley/API/Teams/Features.hs Show resolved Hide resolved

battermann reviewed May 24, 2022

View reviewed changes

services/galley/src/Galley/Cassandra/TeamFeatures.hs Outdated Show resolved Hide resolved

elland temporarily deployed to cachix May 25, 2022 13:16 Inactive

fisx reviewed May 27, 2022

View reviewed changes

elland marked this pull request as ready for review May 30, 2022 07:37

elland force-pushed the add-ttl-to-feature-api branch from 048e605 to 5f7acac Compare May 30, 2022 08:12

elland temporarily deployed to cachix May 30, 2022 08:12 Inactive

elland added 6 commits May 31, 2022 12:44

Added TTL query field to features API.

8440103

Added integration test for TTL.

4629e84

Extracted repeating functions.

34cc03f

Replaced _ with _ttl for readability.

7c6e8bf

Renamed a function for clarity.

f87ed0a

Fail more gracefully if we receive malformed data for TTL.

c93ec1d

elland force-pushed the add-ttl-to-feature-api branch from 5f7acac to c93ec1d Compare June 1, 2022 07:52

elland temporarily deployed to cachix June 1, 2022 07:52 Inactive

Updated changelog.d

5c05ac8

elland temporarily deployed to cachix June 1, 2022 07:58 Inactive

fisx reviewed Jun 1, 2022

View reviewed changes

changelog.d/1-api-changes/added-ttl-param-to-back-office-feature-api Outdated Show resolved Hide resolved

libs/wire-api/src/Wire/API/Team/Feature.hs Outdated Show resolved Hide resolved

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

fisx reviewed Jun 1, 2022

View reviewed changes

libs/wire-api/src/Wire/API/Routes/Public/Galley.hs Show resolved Hide resolved

Updated changelog.

31f2697

elland temporarily deployed to cachix June 1, 2022 11:09 Inactive

elland temporarily deployed to cachix June 1, 2022 14:11 Inactive

Updated default from 8w to unlimited.

c77f5f7

elland force-pushed the add-ttl-to-feature-api branch from 7eec2ff to c77f5f7 Compare June 1, 2022 14:14

elland temporarily deployed to cachix June 1, 2022 14:15 Inactive

Implemented ToHttpApiData in terms of ToByteString.

1f41aab

elland temporarily deployed to cachix June 1, 2022 15:03 Inactive

fisx reviewed Jun 2, 2022

View reviewed changes

tools/stern/src/Stern/API.hs Outdated Show resolved Hide resolved

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

services/galley/test/integration/API/Teams/Feature.hs Show resolved Hide resolved

fisx reviewed Jun 2, 2022

View reviewed changes

services/galley/test/integration/API/Teams/Feature.hs Outdated Show resolved Hide resolved

Actually convert days to seconds as docs imply.

ddd7b2c

Sad: stern is not tested, so we did not catch this with a test.

elland temporarily deployed to cachix June 2, 2022 12:04 Inactive

elland added 2 commits June 2, 2022 12:10

Removed redundant comment.

8f816fe

Uncommented test assertion.

fe53bc1

elland temporarily deployed to cachix June 2, 2022 12:19 Inactive

Implemented FromHttpApiData in terms of FromByteString.

4afdbe8

elland temporarily deployed to cachix June 2, 2022 12:41 Inactive

fisx added 3 commits June 5, 2022 20:55

Add roundtrip test.

b16fe5d

Remove unused aeson instances.

4be1726

Fix cql instance.

8cda21e

fisx temporarily deployed to cachix June 5, 2022 18:58 Inactive

fisx added 2 commits June 5, 2022 21:12

Less misleading naming.

d238cf3

Layout nit-pick.

961e22b

fisx temporarily deployed to cachix June 5, 2022 19:12 Inactive

fisx approved these changes Jun 5, 2022

View reviewed changes

libs/wire-api/src/Wire/API/Team/Feature.hs Outdated Show resolved Hide resolved

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

tools/stern/src/Stern/Intra.hs Show resolved Hide resolved

Added clarifying comment.

030147b

elland temporarily deployed to cachix June 7, 2022 07:46 Inactive

fisx approved these changes Jun 8, 2022

View reviewed changes

elland merged commit f3411cf into develop Jun 8, 2022

elland deleted the add-ttl-to-feature-api branch June 8, 2022 07:41

battermann mentioned this pull request Jun 8, 2022

Release 2022-06-08 - (expected chart version 4.13.0) #2461

Merged

Comments

Conversation

elland commented May 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

elland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

battermann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fisx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fisx commented May 31, 2022

Uh oh!

fisx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fisx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

elland commented May 19, 2022 •

edited

Loading