Elevate missing .shed.yml to error

[hexylena]

From #107 (comment)

This change will probably cause repositories which passed linting before, to fail now, if people were passing directories which weren't things which should be uploaded to the tool shed. Is that acceptable, or should this be hidden behind a strictness falg or something?

[peterjc]

How will this interact with recursion and different levels of testing for tools? e.g. tools/tool_a/ vs tools/suite_b/tool_c/ where not all the folders should have a .shed.yml file.

[hexylena]

http://galaxy-iuc-standards.readthedocs.io/en/latest/best_practices/repositories.html#github-repositories according to this, suites should be in a separate directory, which sidesteps the problem ;)

But yes, you raise a good point. I don't have a good answer for this, and I'm open to ideas. I just believe that a missing .shed.yml is super important. worth throwing a large error for and forcing people to re-organize their workflows, since it seems like tests silently pass when they shouldn't, and a false positive here is quite bad.

[peterjc]

I agree this is important, and as long as it doesn't break existing widespread folder structures, its a very good thing. Planemo already has some non .shed.yml file base tool detection, but I don't know exactly how that works.

[jmchilton]

I was a bit confused by this at first - because I thought planemo defines a repository as a directory containing a .shed.yml file - and for the most part it does. But the shed operations it looks like will just use the supplied path as is if it doesn't find any .shed.yml files.

def _find_raw_repositories(ctx, path, **kwds):
    name = kwds.get("name", None)
    recursive = kwds.get("recursive", False)

    shed_file_dirs = find_matching_directories(
        path, SHED_CONFIG_NAME, recursive=recursive
    )

    config_name = None
    if len(shed_file_dirs) == 1:
        shed_file_dir = shed_file_dirs[0]
        try:
            config = shed_repo_config(ctx, shed_file_dir, name=name)
        except Exception as e:
            error_message = PARSING_PROBLEM % (shed_file_dir, e)
            exception = RuntimeError(error_message)
            _handle_realization_error(exception, **kwds)
            return [exception]
        config_name = config.get("name", None)

    if len(shed_file_dirs) > 1 and name is not None:
        raise Exception(NAME_INVALID_MESSAGE)
    if config_name is not None and name is not None:
        if config_name != name:
            raise Exception(CONFLICTING_NAMES_MESSAGE)
    raw_dirs = shed_file_dirs or [path]

This behavior is because of the last line here. I really was trying to take a very light touch with .shed.yml files originally and everything in there truly was optional - non-optional fields could be specified at the command-line for instance.

With that in mind, two things.

• I don't think a missing .shed.yml would be caught by tools-iuc's Travis setup with this change since the jq directory wouldn't have been picked up as a directory/repository to lint. So I'm not sure this helps JQ: add shed.yml file tools-iuc#1402. Maybe I'm wrong though?
• Following the linting levels for tools - in my view of things this isn't an error, it is a warning - the repository can still be used - it isn't broken per se. Error the way I've tried to use it (and maybe I've failed to be consistent at times) means usable, completely broken - not just in poor form.
• If we change this from error to warn, the check becomes a much weaker check than what is produced with --ensure_metadata and that makes sense I think - the whole point of .shed.yml is provide some default metadata properties.

Therefore - I think the existing INFO or WARN would be correct - the problem is that you are linting in such a way that you are explicitly ignoring warnings. I think it would be better to just fail on warnings as is the planemo default. I've opened a tools-iuc PR here galaxyproject/tools-iuc#1407 that would cause metadata problems with repositories to fail the build while still "ignoring" warnings in the tools themselves - I think that is better right?