TASK: Remove --reset-projections flag from cr:setup command

[bwaidelich]

Removes the --reset-projections option from the cr:setup command because that could fail

Fixes: #5008

[mhsdesign]

Thanks for implementing the stuff i complain about but now i understood what you thought and wrote at the other place. I simply ruled out this as this will defeat the purpose of --reset-projections. The flag is meant as hardcore fix in case we introduce a new schema config that cannot! be setup'd without having to empty the projections (because the content cant be migrated) see christians integer change etc.

Now the flag works well for this case, but if also a new table is introduced the reset will complain due to the table not existing.
Its a henn egg problem. Cant do any of them first. Thus i wanted to do a forgiving reset - which should be imo fine.

[bwaidelich]

@mhsdesign can you elaborate in which cases a setUp() would break? IMO it should never. What is "christians integer change"?

[kitsunet]

The relation anchors, that required an empty DB or no tables to exist as the existing table was incompatible with the new schema.

[bwaidelich]

The relation anchors, that required an empty DB or no tables to exist as the existing table was incompatible with the new schema.

Ah, right.
But I stick to my claim: IMO setUp() should never fail. If the SchemaDiff can't handle it automatically we can add some custom statements to make it stable.

In general: I want to be able to run ./flow cr:setup in my CI

[mhsdesign]

sure for ci it must always work when were 9.0 stable. but currently --reset-projections was a nice bruteforce introduced with #4864

We can remove this flag again as well but we thought at that time it was a good idea, or have we all forgotten :D

[bwaidelich]

I'm OK with the flag and I'm confident that we can keep setUp non-crashing from now on

[mhsdesign]

TLTR; remove it plz

I'm confident that we can keep setUp

haha no xD flow cr:setup --reset-projections will fail again soon. Once we introduce a new table for the content graph projection again with #5096

but that shows surely that it was a bad idea from me to introduce it ^^

though having our beta users have to run

flow cr:resetall --content-repository default // ignore the error it yields
flow cr:setup --content-repository default
flow cr:replayall --content-repository default

was also not cool thats why we introduced it. But it can go now idk. We dont plan any changes like #4791 anymore

[bwaidelich]

flow cr:setup --reset-projections will fail again soon. Once we introduce a new table for the content graph projection again

Why should that fail??
setUp() will just create the new table, that's what it's there for – Or what am I missing?

Anyways, I'm fine with removing the flag – usually you want to trigger a catchup after reset anyways

[mhsdesign]

hen egg situation as said. One must basically now if this flag should be used now or not.

setUp() will just create the new table, that's what it's there for – Or what am I missing?

in this case a reset before a setup will fail.

But changing the order will not fix the original problem this flag tried to solve see #4791 (comment).

But as we dont want any breaking changes anymore in the schema that cant be automigrated with filled data (asking @kitsunet here?) we can drop it

[mhsdesign]

sorry that everything i touch grows into a large discussion 😅

[mhsdesign]

cant we just share one singleton instance of my brain part that holds the cr parts :D

[kdambekalns]

Sounds like a dangerous thing anyways… 😎