Replace OS-based inbox with "programmatic" inbox

NBISweden / LocalEGA

Please go to to https://github.com/EGA-archive/LocalEGA instead

Apache License 2.0

4 stars 1 forks source link

Replace OS-based inbox with "programmatic" inbox #316

Closed dtitov closed 6 years ago

dtitov commented 6 years ago

Replacing Unix SSHD with Apache Mina SSHD (https://mina.apache.org/sshd-project/):

no need for PAM code (C);
no need for FUSE or cron as file events are handled out of the box;
possibility to integrate verify feature to this microservice;
possibility to make on-the-fly files streaming to S3 without intermediate storage.

Sources to review: https://github.com/NBISweden/LocalEGA-inbox

At the moment it doesn't include ingestion functionality (doesn't put the file directly to S3), but rather just replaces existing inbox (the "dummy" one). But later if we implement direct streaming to S3, the ingestion functionality will be absorbed by that.

blankdots commented 6 years ago

This has my blessing :church:, functionality seems to be there. Docs to be updated in a different PR. @juhtornr please also review LocalEGA-inbox code then: :package: :rocket:

silverdaz commented 6 years ago

It sends a message to the broker on close, but it is a slightly wrong logic: The "on close" event should not be solely caught. It must be captured along with the create/append event. Simply reading the file will trigger an open/close sequence that will reprocess the file. See here

dtitov commented 6 years ago

I'll check if reading the file triggers it, thanks.

dtitov commented 6 years ago

You were right. I'll make a PR to fix it soon.

silverdaz commented 6 years ago

We have faced these issues before. This is redoing work that was done before. What if I had not looked at it (out of curiosity this morning) ?

dtitov commented 6 years ago

This is a bug (non-critical bug though, in my mind). We had bugs before and we will have them in the future - it's absolutely natural.
I'm not an oracle, but I would assume that if you haven't had looked at it this morning, we would still find this bug some time later. For instance, I had in mind to make some tests of what happens if a file is uploaded and then removed from the inbox (and some related scenarios e.g. file copying/moving, etc.), maybe even create new tests in the suite for these scenarios.
In some sense, it is a redoing, yes. But such a redoing that solves a specific problem and also gives us some potential benefits, listed above. We can discuss it tomorrow, if you'd like, I can elaborate on that.

Anyway, thank you for spotting this, it was helpful.