-
Notifications
You must be signed in to change notification settings - Fork 45
Don't set expiration on auth tokens #3105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Don't set expiration on auth tokens #3105
Conversation
To make sure that #3096 has the desired effect. [ci] Signed-off-by: Martin Florian <[email protected]>
|
|
|
Deploy cluster test triggered for Commit 885534051d30d06f5f4ec71668d22450d3700460 in , please contact a Contributor to approve it in CircleCI: https://app.circleci.com/pipelines/github/DACH-NY/canton-network-internal/39742 |
|
@moritzkiefer-da So WDYT about this? I think it makes sense but perhaps let's not merge before cutting 0.5.0; it's a low extra risk but why chance it. |
|
lgtm, this sets us back to the state before the canton change so this is quite well tested, I would just merge it even before 0.5.0 |
But not on 3.4? This is what I mean with "low risk"... |
Unless you don't feel strongly about this I'd just add it to #2732 ; don't see a significant gain from merging earlier and I'm already too nervous about this release |
|
feels a bit unnecessary to delay but I don't care enough so your choice |
|
basically at this point I'm "let's not delay this release further > potentially some users might be sad because they have to add an extra field to tokens" (and in reality I don't think it actually matters; surely if CI just passed this will not get broken again... just my nervousness I guess) |
|
I guess I don't understand why clicking "merge" now would delay the release but 🤷 |
Well if, contrary to our expectation, this does cause some of our flows to fail somewhere, we might need to fix it. Also I guess I mean not just the release itself but also the upgrades to it. |
|
it was literally what we had on the main branch until 2 weeks ago so it breaking something seems highly unlikely. but not point in spending more time on it, I'm fine either way |
To make sure that #3096 has the desired effect.
[ci]
Signed-off-by: Martin Florian [email protected]