Project

General

Profile

Actions

Bug #18013

closed

[api] the rake tasks db:check_long_lived_tokens and db:fix_long_lived_tokens should take API.MaxTokenLifeTime into account

Added by Ward Vandewege over 2 years ago. Updated over 2 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Story points:
-
Release relationship:
Auto

Description

These rake tasks only consider the Login.TokenLifeTime configuration value.

We want to use API.MaxTokenLifeTime instead. And if that is not set, use Login.TokenLifeTime, since the docs state that the former takes precedence.


Subtasks 1 (0 open1 closed)

Task #18014: review 18013-token-rake-task-fixResolvedWard Vandewege08/12/2021Actions
Actions #1

Updated by Ward Vandewege over 2 years ago

  • Status changed from New to In Progress
Actions #2

Updated by Ward Vandewege over 2 years ago

  • Description updated (diff)
Actions #3

Updated by Ward Vandewege over 2 years ago

Ready for review at 377467f5285670a797ec02b4ca6c5a129c289a39 on branch 18013-token-rake-task-fix. Tests are running at developer-run-tests: #2635 .

Actions #4

Updated by Lucas Di Pentima over 2 years ago

Just one comment:

  • At line 55 there's a next call that AFAICT makes the rake task return early but in that task, I think the idea is to report so maybe it would make more sense to just display the warning and continue with the iteration?

Apart from that, it LGTM.

Actions #5

Updated by Ward Vandewege over 2 years ago

Lucas Di Pentima wrote:

Just one comment:

  • At line 55 there's a next call that AFAICT makes the rake task return early but in that task, I think the idea is to report so maybe it would make more sense to just display the warning and continue with the iteration?

Apart from that, it LGTM.

Well, that `next` call is inside a loop so it just continues to the next iteration. It's true that the other `next` calls in the rake task make the task return early (they are the equivalent of a `return`). This is a rake task oddity, maybe I should just add a comment along those lines? If that makes sense I can merge this.

Actions #6

Updated by Ward Vandewege over 2 years ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved
Actions #8

Updated by Peter Amstutz over 2 years ago

  • Release set to 42
Actions

Also available in: Atom PDF