-
Notifications
You must be signed in to change notification settings - Fork 51
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
JSON update #1043
JSON update #1043
Conversation
7546d93
to
7c3c985
Compare
201ca25
to
88ac6d1
Compare
44f3bad
to
2ee1ab4
Compare
Warning for unused global options should work now, needs a bit more testing though. I configured an ADIOS2 dataset with the following string: {
"resizable": true,
"asdf": "asdf",
"adios2": {
"unused": "dataset parameter",
"dataset": {
"unused": "too",
"operators": [
{
"type": "blosc",
"parameters": {
"clevel": 3,
"doshuffle": "BLOSC_BITSHUFFLE"
}
}
]
},
"hdf5": {
"this": "should not warn"
}
}
} Resulting warning:
There is no warning for the HDF5 dataset as the HDF5 part is not parsed. |
b783973
to
7f12065
Compare
Only thing really still missing for finishing this PR is to introduce JSON configuration for stuff in ADIOS1 that has so far been configured via environment variables. Since this PR is already large enough, I'll suggest to postpone it to a follow-up.
|
272b16d
to
91c8e5a
Compare
91c8e5a
to
c2007b2
Compare
I've done some cleanup now and the CI is now also running fine |
c2007b2
to
ece54ac
Compare
I merged #1101 now :) |
ece54ac
to
53c1a15
Compare
These are the backends that are already configurable via JSON. New additions: 1. Use of error::BackendConfigSchema 2. Lower case string value reading where applicable 3. Warning when global Dataset options are unused 4. Lax datatypes for string values
Also, warn on unused global keys
The CI doesn't like having too many escaped characters in a normal string
JSON params may be specified in arbitrary capitalization. The backend should recognize this and not read from environment variables if the parameter has already been set via JSON.
Currently those objects that contain keys forwarded to ADIOS2 Can be extended as needed
bbaef55
to
44890dd
Compare
e53d391
to
6065d56
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot 🎉 Great work! 🚀 ✨
Fix #1037, see there for a description.
This will probably not address all of the points noted therein, currently we have:
adios2.engine.usesteps = true
(as a boolean) butadios2.engine.parameters.OpenTimeoutSecs = "30"
(as a string).TracingJSON
everywhere: partially implemented, warnings not yet shown for global optionsDataset
:chunks
,compression
andtransform
. So far, onlytransform
was even implemented (I've replaced that implementation with a JSON-based one now), and only in ADIOS1 which is legacy anyway. For keeping compatibility with user code, we should maybe keep the fields for now, but deprecate them and remove all implementation surrounding them. Discussed: remove them fully in the API if we change the API contract (implementation) to break only once.Are these options in ADIOS2 capitalization insensitive? If not, I'll need to add some handling for that.Eh, let's not rely on this in ADIOS2. I've added exceptions for the lower case transformation now.