Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

[WIP] Generic data recorder #1478

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

jordiandreu
Copy link

Implements a mechanism to register a custom-user generic data recorder. The mechanism enables the possibility to register one or more custom data recorders. The recorders get configured by setting the sardana environment variable DataRecorder with one or more recorder class names.

Documentation about the implementation and configuration of a custom data recorder still missing.

@@ -42,6 +42,10 @@ Sardana provides the following standard recorders (grouped by types):
* JsonRecorder [*]
* OutputRecorder

* generic [*]
.. TODO Need to document the implementation and configuration

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need at least one blank line before the .. TODO comment otherwise travis complains

@reszelaz
Copy link
Collaborator

Thanks @jordiandreu and @tiagocoutinho for working on this PR!
Just a comment on the documentation, there is #1457 which moves around some documentation related to the recorders. It would be nice to get that one integrated first so you work already on the reorganized document.

Also, there is the environment variables catalgue which would need to be updated with the new environment variable.

Regarding the implementation, do you foresee to parametrize somehow the initialization of the recorder class? Like for example the file recorders they receive the path to the file?

@tiagocoutinho
Copy link

tiagocoutinho commented Jan 29, 2021

Regarding the implementation, do you foresee to parametrize somehow the initialization of the recorder class? Like for example the file recorders they receive the path to the file?

Yes, the idea was that each recorder would be responsible to figure out which parameters it needs to configure itself.
In this example, a the recorder looks for a RedisRecorder env to configure itself

@reszelaz
Copy link
Collaborator

Perfect! Thanks for the clarification!

@reszelaz
Copy link
Collaborator

When reviewing issues I just found again #1274, which I think would benefit from this PR. @aureocarneiro, @13bscsaamjad how were you configuring the KafkaRecorder mentioned in #1274 (comment)?

@13bscsaamjad
Copy link

we have found that if you put multiple ScanFile and ScanRecorder, sardana uses them all sequentially. the catch is to have same number of ScanFile as ScanRecorder. we only tested with two though. embedding the example config.

senv ScanFile "['test1', 'test2']" 
senv ScanRecorder "['Scicatrecorder', 'H5Recorder']"

@teresanunez
Copy link

Sorry if I have not understood what you mean, we use at DESY very very oft several recorders and the scanfile is the same
for all of them, only by the termination we know the format. For us it has to continue working like this.

@reszelaz
Copy link
Collaborator

reszelaz commented Mar 1, 2021

@teresanunez there are no plans to change/remove activation of file recorders based on the file extension.
The ScanRecorder variable was added in the past to explicitelly activate a recorder regardless of the one that Sardana would assign based on the file extension. This PR is to propose a mechanism for activating data recorders which do not write to a file.

Thanks for sharing your configuration @13bscsaamjad.
If I understands well the ScanFile value "test1" corresponding to the ScanRecorer value "Scicatrecorder" does not correspond to any real file - this recorder does not write to a file, right? So you used it as a placeholder so there is a match of elements in both lists?

@13bscsaamjad
Copy link

@reszelaz yes exactly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants