rate_limit: Add time unit week

[stefansedich]

Commit Message: rate_limit: Add time unit week
Additional Description:

Following on from #24070 and adding support for week as a unit of time for ratelimits as this is something my team currently needs and WEEK felt sad as it was missing the protobuf party.

Ideally we would have something like #33277 but in the interim it makes sense to add WEEK here since we have every other period already except week.

Risk Level: Low
Testing: Added unit test coverage
Docs Changes: n/a
Release Notes: added
Platform Specific Features: n/a

[repokitteh-read-only]

Hi @stefansedich, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #37494 was opened by stefansedich.

see: more, trace.

[repokitteh-read-only]

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #37494 was opened by stefansedich.

see: more, trace.

[wbpcode]

So, the week is 7 days rather than a natural week, right? If is that way, please add a comment to document it clearly. Thanks.

[stefansedich]

So, the week is 7 days rather than a nature week, right? If is that way, please add a comment to document it clearly. Thanks.

First I have ever heard of a "nature week" 😸 hopefully that comment is what you were after! I guess it is also fitting that the enum value is also 7 here...

[wbpcode]

So, the week is 7 days rather than a nature week, right? If is that way, please add a comment to document it clearly. Thanks.

First I have ever heard of a "nature week" 😸 hopefully that comment is what you were after! I guess it is also fitting that the enum value is also 7 here...

Sorry, I didn't make it clear. But never mind, I just realized that the YEAR here is also not natural year.

So, it's fine, will take a more detailed check tomorrow

[markdroth]

/lgtm api

[ggreenway]

This is failing coverage; please add a test that exercises this.

/wait

[stefansedich]

This is failing coverage; please add a test that exercises this.

/wait

Ok no problems let me see what I can add, had followed the PR that added month/year as a guide which did not add coverage but happy to add something.

[stefansedich]

[----------] Global test environment tear-down
[==========] 12 tests from 1 test suite ran. (2 ms total)
[  PASSED  ] 12 tests.

@ggreenway coverage added, let me know if you have any issues/suggestions with what I did, I have not touched C++ in 15+ years so had to fumble may way through this.

[wbpcode]

ci still not happy

/wait

[stefansedich]

ci still not happy

/wait

Thanks @wbpcode I saw those failures and was not sure as some of them looked like they flaked for unlrelated issues, let me know if I have missed something obvious.

[wbpcode]

/retest

[wbpcode]

ci still not happy
/wait

Thanks @wbpcode I saw those failures and was not sure as some of them looked like they flaked for unlrelated issues, let me know if I have missed something obvious.

let me retry the ci

[wbpcode]

cc @ggreenway

[stefansedich]

Hi @wbpcode @ggreenway is there anything further I can do here to help this one land?