sergehuber commented on PR #867:
URL: https://github.com/apache/unomi/pull/867#issuecomment-5732311720
Thank you, Jonathan — this is a clean diagnosis of a bug that is easy to
miss, and I am grateful you followed the Camel route all the way through.
One-shot import taking the tenant from the parent directory, while the endpoint
wrote at the upload-dir root, fully explains why those files were dropped.
Writing `{uploadDir}/{tenantId}/{importConfigId}.csv` matches that contract
nicely.
Two small suggestions, only if they are easy for you. Neither should be a
lot of work, and I am happy to help:
- A null-check on `executionContextManager.getCurrentContext()` would turn a
missing context into a 4xx instead of an NPE. Something like returning 400/409
when `getCurrentContext()` is null would be plenty.
- This same file is also touched in #853 (`@RequiresRole` on the endpoint).
Rebasing onto #853 (or waiting until it lands) would save a merge conflict. No
rush if you would rather wait.
I would not block on a unit test — `router-rest` has no real harness for
this path today. A one-liner asserting the destination path would be a kindness
to the next person, nothing more.
Thank you again for this. I would be glad to merge it once it sits cleanly
next to #853.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
