Currently the cron key is in the configuration system. However it seems like it more rightly belongs in the state system. It is generated new on every site install, and there is no real use case for deploying the same cron key around a set of sites. When I tried to think through a use case with msonnabaum, he felt that this was not realistic. This would require

- Removing cron_key from system.cron.yml
- Changing the config() calls to state() calls

Files: 
CommentFileSizeAuthor
#19 cron_key_to_state-1821530-19.patch6.9 KBLinL
PASSED: [[SimpleTest]]: [MySQL] 47,804 pass(es). View
#18 cron_key_to_state-1821530-18.patch6.98 KBLinL
PASSED: [[SimpleTest]]: [MySQL] 46,895 pass(es). View
#17 cron_key_to_state-1821530-17.patch6.98 KBLinL
PASSED: [[SimpleTest]]: [MySQL] 46,445 pass(es). View
#16 cron_key_to_state-1821530-16.patch6.99 KBLinL
PASSED: [[SimpleTest]]: [MySQL] 46,473 pass(es). View
#9 cron_key_to_state-1821530-9.patch6.95 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 46,439 pass(es). View
#9 5-9-interdiff.txt1.85 KBalexpott
#5 cron_key_to_state-1821530-5.patch5.38 KBLinL
PASSED: [[SimpleTest]]: [MySQL] 46,224 pass(es). View
#1 cron_key_to_state-1821530-1.patch4.65 KBLinL
FAILED: [[SimpleTest]]: [MySQL] 46,220 pass(es), 1 fail(s), and 0 exception(s). View

Comments

LinL’s picture

Status: Active » Needs review
FileSize
4.65 KB
FAILED: [[SimpleTest]]: [MySQL] 46,220 pass(es), 1 fail(s), and 0 exception(s). View

Status: Needs review » Needs work

The last submitted patch, cron_key_to_state-1821530-1.patch, failed testing.

msonnabaum’s picture

Status: Needs work » Active

Agreed. This does not seem like config. Good use case for state.

msonnabaum’s picture

Status: Active » Needs work

oops.

LinL’s picture

FileSize
5.38 KB
PASSED: [[SimpleTest]]: [MySQL] 46,224 pass(es). View

Missed one, here's another go.

LinL’s picture

Status: Needs work » Needs review
alexpott’s picture

Issue tags: +State system

Tagging...

Berdir’s picture

Status: Needs review » Needs work

Same here, I think this should now use the state update helper function.

alexpott’s picture

FileSize
1.85 KB
6.95 KB
PASSED: [[SimpleTest]]: [MySQL] 46,439 pass(es). View

Patch attached makes change suggested in #8 and adds test.

alexpott’s picture

Status: Needs work » Needs review

Testing...

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, RTBC if it passes the tests.

@Core maintainers: We've RTBC'd quite a few of these conversion issues today. When committing, make sure that update numbers don't clash and if they do, either fix that or set it to needs work. You can alternatively also run update.php before commiting to be sure that everything's fine.

catch’s picture

Status: Reviewed & tested by the community » Needs work

The last submitted patch, cron_key_to_state-1821530-9.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
Issue tags: +Configuration system, +Config novice, +State system

#9: cron_key_to_state-1821530-9.patch queued for re-testing.

heyrocker’s picture

Status: Needs review » Reviewed & tested by the community

Green!

LinL’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
6.99 KB
PASSED: [[SimpleTest]]: [MySQL] 46,473 pass(es). View

Re-roll, patch no longer applied.

LinL’s picture

FileSize
6.98 KB
PASSED: [[SimpleTest]]: [MySQL] 46,445 pass(es). View

And again. Last one had an extra space.

LinL’s picture

FileSize
6.98 KB
PASSED: [[SimpleTest]]: [MySQL] 46,895 pass(es). View

Updating system_update_N

LinL’s picture

FileSize
6.9 KB
PASSED: [[SimpleTest]]: [MySQL] 47,804 pass(es). View

Re-roll, patch no longer applied.

Status: Needs review » Needs work
Issue tags: -Configuration system, -Config novice, -State system

The last submitted patch, cron_key_to_state-1821530-19.patch, failed testing.

LinL’s picture

Status: Needs work » Needs review
Issue tags: +Configuration system, +Config novice, +State system

#19: cron_key_to_state-1821530-19.patch queued for re-testing.

heyrocker’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC here

catch’s picture

Status: Reviewed & tested by the community » Fixed

Thanks! Committed/pushed to 8.x.

Automatically closed -- issue fixed for 2 weeks with no activity.