#30360 -- Support rotation of secret keys

@carltongibson
The test run gives me this
FAILED (failures=1, errors=50, skipped=1169, expected failures=4)

Are the 50 errors expected?

The failing test is this one:

I understand what the test does but I am unsure how to make it pass. I tried to remove the assertRaisesRegex(ImproperlyConfigured, msg) call but the test raises an ImproperlyConfigured error either way.

Hi @terrameijar — no the errors aren’t expected. (Compare running the tests on master…) :grinning:

What’s happened is the patch has made some changes which have broken other changes which were subsequently made. (Florian’s PR that moved the secret key check to first access most likely.) So we need to look at the errors and work out what changed. It’s a puzzle. :slightly_smiling_face:

I’ll give it a run today, and see if I can spot the issue. (Likely it’s one thing causing all the errors.)

Hi @terrameijar.

I had a quick look. The various errors were caused by a bad rebase on tokens.py. I looked at the error, looked at the diff, and the file history, and adjusted to fix it here.

That just leave the SECRET_KEY failures which I think are coming from the block in django/conf/__init__.py — That logic was adjusted to make the check on first access — See my comment on the PR — Have a look at the linked commit there and see if you can work out how to update to match the intent of both commits to resolve. (Hopefully that’s the fun bit — let me know if you need input!)

Once we have the tests passing, we can work on the documentation and release notes — There’s a new setting, and here’s how you use it.

Hey @terrameijar — did you have any bandwidth to get back to this PR as yet? Do you need any input? Do you think you will have capacity? (No stress my end — I just want to make sure you’ve got support that you need.)

Hi @carltongibson, sorry I had not, it has been pretty busy on my end. I had a look and will post an update to the PR with questions later today. I think i’m on the right track but need clarification on one small thing. Thanks

Hey @terrameijar — no problem — you always look like you’re doing loads of stuff! :smile:

I’ve just pushed a commit to the PR to address the failing tests. One test still fails. Please review when you have a moment and let me know if i’m on the right track @carltongibson. Thank you. I appreciate your help with this.

Super. Let me have a look and I’ll get back to you. :grinning:

Hey @carltongibson Were you able to take a look at this? I’ll have some free time next week and would like to work on it. Thank you.

HI @terrameijar, I hope you’re well!

Were you able to take a look at this?

Yes I did. I got so far through updating the SECRET_KEY validation move, and then had to stop. It’s a bit more complex than it was previously, because we need to check both possibilities and validate on first access, and then adjust the tests for those.

It’s on my list to pick up.

I’ll have some free time next week and would like to work on it.

Super! If we assume I’ll resolve the test failure, the thing that’s not there at all (and was really the TODO before all this started) was the Docs and Release Notes changes.

So, (first) to add docs for the SECRET_KEYS setting, and then (second) a release note matching that. If you want to focus there, that would be awesome! Just ping me here or on GitHub if you need input.

For reference, the feature freeze for Django 3.2 is January 14th. That’s our deadline. It has to be merged by then. :smiley:

Thanks for the follow up! Good hustle.

Hey @carltongibson . My apologies for the late response. I really wanted to do this but I haven’t been able to, there’s been a lot happening on my end and I just couldn’t get the time. Let me try again with a different ticket when things settle down for me.

Thank you.

Hey @terrameijar — sorry for the slow reply — I’ve only just been head about water for the last few weeks. :grinning_face_with_smiling_eyes:

No stress at all! Life > OSS. Never forget that. :grinning_face_with_smiling_eyes:

In the end we bumped the ticket to 4.0, so it’s on my list to pick it up over the early part of the cycle. If you’d like to carry on with it, please take a look at the new branch (although it needs rebasing again :slight_smile: — Or, if you fancy working on another ticket, that’s cool too. If you need input on that let me know — but as time and space allow for you — no pressure our end.

Kind regards, keep safe.