Closed hexylena closed 5 years ago
Hey @erasche, it looks like we've always had this as revisions
, at least as far back as https://github.com/galaxyproject/ansible-galaxy-tools/commit/bfdd53c19e3bb17669bb2b3c9409cf6fe59b7e26. The helper scripts produce this under the revisions
key as well. Do you think we should add a changeset_revisions
alias for this ? I think I've replied on the mailing list a few times about this, so it seems this comes naturally (especially since the galaxy API/UI calls this changeset_revision
)
I feel like I'm going crazy when you point that out from 2015. I know I copied changeset_revision
from somewhere, but I don't see it now. No, there's no need to support this, maybe I got confused from the use of changeset_revisions in so many other places.
Ok, no, it's especially odd that ephemeris interprets changeset_revision
as a key which stores a revision. Thinking about it, I would expect an invalid key to be ignored, but instead this invalid key is parsed (incorrectly) into trying to be a revisions
that should be installed. I'd argue that that specific behaviour is unexpected.
It looks like the flattening function is the culprit:
_strip_revisions
which doesn't strip the conflicting key changeset_revisions
changeset_revisions
is then used internally in a "flat" version of the reporevisions
, we hit the else
, and the repo is appended as-is, and changeset_revisions
is set to an illegal value because it was never checked.Due to a 'copy' of the dict, rather than creating a new one with only selected / validated properties passed through.
Yep, that's what I figured as well. Why don't we strip all invalid keys and log them as a warning ?
Sounds good to me!
Hey @erasche . Quite some changes have been made to shed-tools with #104 . Does this issue still persist? If so, I can implement @mvdbeek's suggestion.
@rhpvorderman the issue would still persist with the refactoring I'm afraid. Please either strip invalid keys or silently drop them, that would be very helpful since this is unvalidated input.
Hmm, we could add a .yaml linter with pykwalify at some point. That might be nice to have.
I'm experiencing some issues installing tools. This had worked $some_time_ago, but now the installation log is completely full of errors like:
the last time I saw this it was because
changeset_revision
was being interpreted as a list and then cast to a string, and then ephemeris was searching for that and failing to find it (because of course there is no revision matching the stringu"['asdfasdfa']"
.Any ideas on this?
Here is the full console log https://build.usegalaxy.eu/job/usegalaxy.eu/job/install-tools/12/consoleFull
It runs the command
make install
from our repo which runs the commandshed-tools install --toolsfile $file.yaml --galaxy $url --api_key $api_key
.Running locally with increased logging level:
This sticks out to me:
I really expected to see something like
Changing the
changeset_revision
torevisions
in our yaml files looks like it fixes it, but it used to work. Did something change recently in the handling of changeset_revision to force it into a list?