enonic / cli-enonic

GNU General Public License v3.0
41 stars 2 forks source link

keep-alive in `cms reprocess` #179

Closed alansemenov closed 4 years ago

alansemenov commented 5 years ago
root@test:/var/enonic-xp/xxx/deploy# enonic cms reprocess
Enter target content path (<branch-name>:<content-path>): master:/
Reprocessing...Unable to connect to remote service: Post http://localhost:4848/content/reprocess: net/http: request canceled (Client.Timeout exceeded while awaiting headers)

Tips from the customer:

enonic-cli uses net/http synchronously and will time out after 1m on i.e. enonic cms reprocess. This is an obvious design issue, needs to either async/long-poll or tcp keepalive. For instance a cms reprocessing can easily take more than 1min:

pmi commented 5 years ago

CLI is able to make async task polling, but that requires server command to be an async task with progress reporting.

Currently following commands are tasks and polled by CLI:

So the bottom line is, ContentResource.reprocess() needs to support polling first for the CLI to use it.

alansemenov commented 5 years ago

@GlennRicaud any objections against adding polling support to reprocess?

GlennRicaud commented 5 years ago

Sounds good. But when doing these changes in the CLI, keep in mind that.

So the best would be a new endpoint here to not break compatibility (and point 2 above).

sigdestad commented 5 years ago

We do not need old CLI to work with newer XP's.

GlennRicaud commented 5 years ago

I did not think it was an internal API. I have seen customers use this API with their own script. Breaking it is not advisable.

pmi commented 5 years ago

Created 2 PRs in cli-enonic as well as xp repos with new reprocess method instead of the old one. Let me know if we need a new reprocess method along with the old one to keep backwards compatibility. https://github.com/enonic/cli-enonic/pull/180 https://github.com/enonic/xp/pull/7518

alansemenov commented 5 years ago

This one has to be updated as well, after updates to enonic/xp#7518

pmi commented 4 years ago

Updated PR