Add resolvers in file loader

[TeresiaOlsson]

This PR is for issue #310.

I have added the possibility to have different resolvers in the loader. Currently only two exists for env and file. The syntax for both of them is the same ${resolver:value}.

The old syntax file: has been removed and all test configuration files updated to instead use the new syntax.

It is still supported to put a file without any syntax and it will be resolved as long as it has the file extension is in the ACCEPTED_SUFFIXES list.

[TeresiaOlsson]

The tests are currently failing because many of the configuration files includes a reference to tune_response.json which doesn't exist in the repository at the moment. Should this file be added?

The reason why the tests didn't fail before is because the behaviour has changed slightly in this PR. Now for ${file:} the file is loaded and the contents appended to the configuration. Previously it seems like file: only resolved the path but didn't load the file. Which behaviour is the preferred one?

[TeresiaOlsson]

The FILE_PREFIX is kept at the moment because it is used by the restfetcher and didn't entirely understand how that module works so I didn't dare to change it. It however looked to me like maybe the new resolvers could be used there too?

[kparasch]

The tests are currently failing because many of the configuration files includes a reference to tune_response.json which doesn't exist in the repository at the moment. Should this file be added?

The reason why the tests didn't fail before is because the behaviour has changed slightly in this PR. Now for ${file:} the file is loaded and the contents appended to the configuration. Previously it seems like file: only resolved the path but didn't load the file. Which behaviour is the preferred one?

I think we should not need the file to exist or you end up with circular problems.

To generate the tune response, you need a pyaml configuration.
To load the pyaml configuration you need tune response.

And if you already have a tune_response saved, you shouldn't need to generate it each time.

[TeresiaOlsson]

The tests are currently failing because many of the configuration files includes a reference to tune_response.json which doesn't exist in the repository at the moment. Should this file be added?
The reason why the tests didn't fail before is because the behaviour has changed slightly in this PR. Now for ${file:} the file is loaded and the contents appended to the configuration. Previously it seems like file: only resolved the path but didn't load the file. Which behaviour is the preferred one?

I think we should not need the file to exist or you end up with circular problems.

To generate the tune response, you need a pyaml configuration. To load the pyaml configuration you need tune response.

And if you already have a tune_response saved, you shouldn't need to generate it each time.

Ah, I see what you mean. The configuration actually has two different type of files and they should be handled differently:

1. Files that contains some data but that data should not be loaded in as part of the configuration. The configuration should only contain the path to the file and loading the data will happen later during runtime.

2. Files that contains subsections of the configuration. These should be resolved during the loading.

My question is then why for the tuning tools the first category is using the file: syntax and not doing the same as for the excitation curves? For those there is an attribute file which contains the path. Why for the tuning tools isn't there just response_matrix which should point to a path? What is the purpose of including file: as part of the string?

[kparasch]

The tests are currently failing because many of the configuration files includes a reference to tune_response.json which doesn't exist in the repository at the moment. Should this file be added?
The reason why the tests didn't fail before is because the behaviour has changed slightly in this PR. Now for ${file:} the file is loaded and the contents appended to the configuration. Previously it seems like file: only resolved the path but didn't load the file. Which behaviour is the preferred one?

I think we should not need the file to exist or you end up with circular problems.
To generate the tune response, you need a pyaml configuration. To load the pyaml configuration you need tune response.
And if you already have a tune_response saved, you shouldn't need to generate it each time.

Ah, I see what you mean. The configuration actually has two different type of files and they should be handled differently:

1. Files that contains some data but that data should not be loaded in as part of the configuration. The configuration should only contain the path to the file and loading the data will happen later during runtime.
2. Files that contains subsections of the configuration. These should be resolved during the loading.

My question is then why for the tuning tools the first category is using the file: syntax and not doing the same as for the excitation curves? For those there is an attribute file which contains the path. Why for the tuning tools isn't there just response_matrix which should point to a path? What is the purpose of including file: as part of the string?

Excitation curves could also be annotated with "file:", but there is was never a need to do it because the .csv extension cannot be mistaken for a configuration file.

I agree "file:" itself is a bad name. The idea was that "file:" represents the file-path itself and is not interpreted as a configuration file and the expansion is skipped. If we would put just the "response_matrix.json" path, then the file loader would interpret it as configuration, try to look for a "type: " entry and then fail to load because it doesn't have one. On this point I don't understand your implementation of the resolve_file function. The point would be to actually not load the file, while the function loads it.

Generally I like the concept of the resolvers. The present implementation also gives a very obvious way to configure pyaml using a database:

from pyaml.configuration.fileloader import resolver

@resolver('db')
def resolve_db(name):
    # fetch configuration from database entry
    return name

from pyaml.configuration.fileloader import RESOLVERS
print(RESOLVERS) # db is registered in the RESOLVERS

[TeresiaOlsson]

Okay. Then I understand. I had totally misunderstood the purpose of file:. The purpose of it is just to add the root path to make an absolute path out of a relative one. And not to load anything.

Then I should have a separate resolver for resolving subconfigurations. For those the file should be loaded and the content appended.

At the moment I have mixed those two things into the same resolver.

[TeresiaOlsson]

After some thinking, I have a new suggestion for the syntax of the resolvers:

• ${env:}: resolve an environment variable

• ${path:}: resolve a file path. This is the previous file:. It makes an absolute path of a relative path but doesn't load the file.

• ${file:}: resolve a file. This loads the file and appends the content. For the supported file extensions this resolver will also be used without having to explicitly write the prefix for convenience but I think implementing it as a resolver similar to the others is still a nice idea.

[JeanLucPons]

After some thinking, I have a new suggestion for the syntax of the resolvers:

• ${env:}: resolve an environment variable
• ${path:}: resolve a file path. This is the previous file:. It makes an absolute path of a relative path but doesn't load the file.
• ${file:}: resolve a file. This loads the file and appends the content. For the supported file extensions this resolver will also be used without having to explicitly write the prefix for convenience but I think implementing it as a resolver similar to the others is still a nice idea.

This is OK for me.

[gubaidulinvadim]

@TeresiaOlsson I think the proposed change is good. Please also check the config files in the examples (BESSY, SOLEIL, ESRF, etc.). They might require changes and currently the tests do not check if the changes broke the examples.

[gupichon]

@TeresiaOlsson I think the proposed change is good. Please also check the config files in the examples (BESSY, SOLEIL, ESRF, etc.). They might require changes and currently the tests do not check if the changes broke the examples.

Yes, in the PR #306 I added a simple loading test for those configuration files, so the test will fail if they are not up to date.

[TeresiaOlsson]

I have added a separate resolver for path so the tests are working now. I also updated the examples and removed the FILE_PREFIX since it's no longer used by the file loader. It is however used by the restfetcher and the solution I added there might be a bit temporary and could be done in a better way in the future.