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
StatusFileSize
new4.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

StatusFileSize
new5.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

StatusFileSize
new1.85 KB
new6.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
StatusFileSize
new6.99 KB
PASSED: [[SimpleTest]]: [MySQL] 46,473 pass(es).
[ View ]

Re-roll, patch no longer applied.

LinL’s picture

StatusFileSize
new6.98 KB
PASSED: [[SimpleTest]]: [MySQL] 46,445 pass(es).
[ View ]

And again. Last one had an extra space.

LinL’s picture

StatusFileSize
new6.98 KB
PASSED: [[SimpleTest]]: [MySQL] 46,895 pass(es).
[ View ]

Updating system_update_N

LinL’s picture

StatusFileSize
new6.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.