Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility with file format version 1.1 #198

Merged
merged 1 commit into from Jul 9, 2021
Merged

Conversation

philsmt
Copy link
Contributor

@philsmt philsmt commented Jun 30, 2021

As learned during a deployment test of the new pclayer version, it now writes an additional row in the beginning of METADATA/dataSources datasets, namely:

  • An empty string in METADATA/dataSources/root
  • The string Karabo_TimerServer in METADATA/dataSources/dataSourceId and METADATA/dataSources/deviceId

As the entry in METADATA/dataSources/dataSourceId contains no valid prefix (either CONTROL or INSTRUMENT), it breaks any and all reading code. This PR makes FileAccess._read_data_sources ignore this entry, checked successfully against data taken during the deployment test.

As far as I can see, we do not read data from this group anywhere else.

EDIT: Now with format version 1.2 deployed, I've changed this PR to limited compatibility with 1.1, see below.

@takluyver
Copy link
Member

Thanks! Is this specific name hardcoded in the DAQ? There's no chance that we'd hit something similar with other names?

@takluyver
Copy link
Member

To record the answer to my question: yes, this name is quite distinctly hardcoded into the machinery that writes the file:

https://git.xfel.eu/gitlab/karaboDevices/pcLayer/blob/1.10.3-2.10.5/src/devices/DataAggregator/Writer.cc#L1633

We're waiting to get official word of whether this is deliberate before we merge this and release.

@philsmt
Copy link
Contributor Author

philsmt commented Jul 5, 2021

I'll keep this MR around until we see the new pclayer tag that no longer needs it.

@philsmt philsmt changed the title Ignore malformed data source occuring with newer pclayer versions WIP: Ignore malformed data source occuring with newer pclayer versions Jul 5, 2021
@takluyver
Copy link
Member

I'm wondering if we'll want to merge it anyway, so that the files created in the meantime can be opened. It should be harmless for the other files.

@philsmt
Copy link
Contributor Author

philsmt commented Jul 5, 2021

Maybe, I did not consider the files already written.

We can hope though that it did not extend beyond some test files during deployment testing, and simply pretend this never happened. Otherwise, it might even be useful to bump the format version number again to sort out the "broken" flag dataset.

@philsmt philsmt changed the title WIP: Ignore malformed data source occuring with newer pclayer versions Compatibility with file format version 1.1 Jul 9, 2021
@philsmt
Copy link
Contributor Author

philsmt commented Jul 9, 2021

I've changed the wording and added a warning when the validation flag is used. I've also added basic compatibility with 1.0-style validation by inverting the INDEX/flag dataset,.

extra_data/file_access.py Outdated Show resolved Hide resolved
@takluyver
Copy link
Member

LGTM 👍

@philsmt
Copy link
Contributor Author

philsmt commented Jul 9, 2021

Thanks for the review!

When we implement some API for the 1.2 semantics to make use of specific source to use for validation, we might be able to also apply this to 1.1, but treating it like 1.0 is probably enough.

@philsmt philsmt merged commit e1add68 into master Jul 9, 2021
@takluyver takluyver added this to the 1.7 milestone Aug 2, 2021
@takluyver takluyver deleted the fix-broken-metadata branch August 2, 2021 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants