Error in pipeline

[OvermindDL1]

Adding this as a PUT resource causes this error in the latest versions of Concourse:

json: cannot unmarshal object into Go value of type []atc.Version

It looks like the check and in files need a more filled out json object? (EDIT: And probably resource.py too)

[aequitas]

Afaik nothing changed in the resource protocol in recent versions. Can you share the configuration and the output when using RESOURCE_DEBUG=1 environment variable?

[OvermindDL1]

It seems that it only does it on initial add, even when only used as a PUT, the system ran its 'polling' once to acquire the version and it's never done it since. Where would I add a RESOURCE_DEBUG environment variable, the params for the resource, or somewhere else? I'm still learning the system as I'm still setting it up (used to use jenkins on an old server...).

[OvermindDL1]

Hmm, no it was paused, it seems to check every few minutes and it keeps reporting the same above error, thus causing it to show as yellow in the pipeline view even though it runs green as the PUT action.

[aequitas]

Sorry it has been a while since I looked at this codebase, the RESOURCE_DEBUG variable is only used for the test scripts and local debugging. When running inside concourse you need to set the debug field in source to true https://github.com/aequitas/concourse-http-api-resource/blob/master/assets/resource.py#L55

[OvermindDL1]

Well that leaked some secrets into the log, good thing I'm wiping it all soon... ^.^;

Where will it print the data? When the prior task in the pipeline runs it still 'puts' fine, along with printing a lot more information (the environment I passed in and the json and so forth, nothing with 'version'), but waited a couple of minutes for the resource itself to 'poll' (once every 2 minutes it seems) and it is still reporting just json: cannot unmarshal object into Go value of type []atc.Version with no extra debug data at all, as well as it doesn't look like the check script would print anything anyway as all it is, is just: https://github.com/aequitas/concourse-http-api-resource/blob/master/assets/check

#!/bin/sh

# empty version to keep Concourse happy
echo '{"version": {}}'

So I'm unsure what I would be expecting it to print for debugging information?

From what little I know about Go, it 'looks' like that check script should actually be just echo '[]' for two reasons, first is that it is trying to serialize the object {"version": {}} into the []atc.Version array, and an object can't go into an array, and second that atc.Version wouldn't follow that format anyway.

Overall it is not affecting functionality, the pipeline still 'works' it is just reporting the pipeline with a yellow arrow because the checking phase of this is failing, which makes it look like a broken pipeline. In addition, even though in is unused and out doesn't version check, both of those should be updated to the same as well? If I'm reading this right of course...

[OvermindDL1]

Looking in the Concourse golang source some more (I'm becoming less a fan of this language as time goes by it seems, poorly designed...) it looks like the format it expects is [{"timestamp": 1234567890}] with the objects in the list being optional. If the list is empty then it will never trigger anything (which is what a resource like this should do I'd guess), otherwise it triggers whenever the timestamp value changes.

[OvermindDL1]

And yet in other areas it looks like it should be time instead of timestamp, huh... Regardless, not used, it can be an empty array.

You know, how about I just check the other plugins that I'm using that do work fine in that way:

• irc-notifier: returns just []
• The official time resource: returns an array of version object with a time key, looks like [{time: 3437298423}]
• The official semver resource: returns an array of version object with a number key, looks like [{number: "1.2.3}]

So it looks like the Version object can hold a variety of fields, any of which changing will cause an update to propagate, but it is always wrapped up into an array and it is that main array that is returned directly, no wrapping object.

This is with concourse version latest release at the time of this post: 4.2.1 However it doesn't really look like this code in the main repository has changed form in a very very long time, so I'm unsure how returning the object ever worked?

[OvermindDL1]

It looks like this repo about 8 months back had the check script correct. I'm not seeing any older versions of this repo on https://hub.docker.com/r/aequitas/http-api-resource/tags/ (only latest? Where are the rest?) so I'm unable to test that old version.

[aequitas]

This resource only supports put, so check and in should do nothing. But maybe there where some changes lately to concourse that now require the behaviour of check and in to different, where previously it would be ignored.

I'll check tomorrow when I have an concourse instance available to test against.

[aequitas]

I believe the previous version of check broke something else (see https://github.com/aequitas/concourse-http-api-resource/pull/12, https://github.com/aequitas/concourse-http-api-resource/issues/8).

I'll have to read upon the spec and do some testing.

[OvermindDL1]

This resource only supports put, so check and in should do nothing. But maybe there where some changes lately to concourse that now require the behaviour of check and in to different, where previously it would be ignored.

Ah that is possible. It definitely is not set to trigger, but the check script is being called every 2 minutes...

I'll have to read upon the spec and do some testing.

There's a spec?! o.O

Searching.... best I can find is this: https://concourse-ci.org/implementing-resources.html

What it says about the check script, emphasis mine:

It is given the configured source and current version on stdin, and must print the array of new versions, in chronological order, to stdout, including the requested version if it's still valid.

I.E. It is passed 'in' on stdin an object like what your check script is currently passing 'out', and it is supposed to pass 'out' on stdout an array of Version objects in chronological order. So yep, the current check script is not following the 'spec' in as much as this is a spec.

It also states (again emphasis mine):

The list may be empty, if there are no versions available at the source. If the given version is already the latest, an array with that version as the sole entry should be listed.

Thus it should have been and always should have been echo '[]' in the shell script, I'm unsure how the current version ever worked?

[OvermindDL1]

Ah, I think I see what they did.

Based on reading https://concourse-ci.org/implementing-resources.html in detail

1. check should return [] and as such it is currently incorrect.
2. in should return {"version": {}} when there is nothing to do, which the current script is doing properly.
3. out is working so I didn't even look at it.

So it looks like whoever PR'd that change only meant to change in and not check too. They fixed in, but broke check. Though, who'd be using in/get on this resource anyway. ^.^;

[aequitas]

If changing the check behaviour back works for you let me know, it's not hard to get your own resource published, just fork this repo and enable automated build for it on docker hub, then you can use it in your concourse. Feel free to create a PR (don't forget to adjust the tests as well).

Otherwise I will try it out tomorrow and revert if needed.

[OvermindDL1]

If changing the check behaviour back works for you let me know, it's not hard to get your own resource published, just fork this repo and enable automated build for it on docker hub, then you can use it in your concourse. Feel free to create a PR (don't forget to adjust the tests as well).

Trying to run tests but I'm getting some 'fun' errors... And this is with no changes yet...

# snip
  Running setup.py install for MarkupSafe: started
    Running setup.py install for MarkupSafe: finished with status 'done'
  Running setup.py install for pycparser: started
    Running setup.py install for pycparser: finished with status 'done'
  Running setup.py install for brotlipy: started
    Running setup.py install for brotlipy: finished with status 'done'
Successfully installed Flask-1.0.2 Jinja2-2.10 MarkupSafe-1.0 atomicwrites-1.2.1 attrs-18.2.0 blinker-1.4 brotlipy-0.7.0 cffi-1.11.5 click-6.7 cookies-2.2.1 decorator-4.3.0 httpbin-0.7.0 isort-4.3.4 itsdangerous-0.24 mccabe-0.6.1 more-itertools-4.3.0 pluggy-0.7.1 py-1.6.0 pycodestyle-2.4.0 pycparser-2.19 pydocstyle-2.1.1 pyflakes-2.0.0 pylama-7.4.3 pytest-3.8.1 pytest-httpbin-0.3.0 raven-6.9.0 responses-0.9.0 six-1.11.0 snowballstemmer-1.2.1 werkzeug-0.14.1
Traceback (most recent call last):
  File "/usr/local/bin/pylama", line 7, in <module>
    from pylama.main import shell
  File "/usr/local/lib/python3.7/site-packages/pylama/main.py", line 10
    from .async import check_async
              ^
SyntaxError: invalid syntax
The command '/bin/sh -c /opt/resource-tests/test.sh' returned a non-zero code: 1

Loading all of python just for an http image seems a bit heavy, a slimmed alpine image with curl and jq installed should do just fine with a shell script for all equivalent functionality I'd think?

[OvermindDL1]

Downgrading python fixed it.

EDIT: Snip, it looks like the bloat is mostly just python itself, didn't realize it was so huge... o.O

[OvermindDL1]

Yeah I can't get the tests to pass, I fix one thing and more things just end up not working, now I'm at one complaining about a lack of CFFI compiler... I'm stripping out the entire testing framework so I can actually test it...

[OvermindDL1]

Wooo!

So this diff:

diff --git a/Dockerfile b/Dockerfile
index 9bdacdc..e42aa2b 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,11 +1,9 @@
-FROM python:3
+FROM python:3.4.9-alpine3.8

 # install requirements
-ADD requirements*.txt setup.cfg ./
+ADD requirements.txt setup.cfg ./
 RUN pip install --no-cache-dir -r requirements.txt

 # install asserts
 ADD assets/ /opt/resource/
-ADD test/ /opt/resource-tests/

-RUN /opt/resource-tests/test.sh
diff --git a/assets/check b/assets/check
index c7ba2c4..0cf7b77 100755
--- a/assets/check
+++ b/assets/check
@@ -1,4 +1,4 @@
 #!/bin/sh

 # empty version to keep Concourse happy
-echo '{"version": {}}'
+echo '[]'

Works perfectly. It's on dockerhub as overminddl1/concourse-http-api-resource-python if you wish to try it. It fixes the bug in the check part of the pipeline, everything worked well, and the image is less than a tenth of it's prior size (every tens of megs I can get is very useful to me). :-)

[OvermindDL1]

I havn't looked at the tests, but if they are testing against an actual concourse instance then I don't know how the previous check call was ever passing the tests at all... It's working exceedingly well now.

[OvermindDL1]

Obviously stripping out the tests would not make for a good PR, and since I can't get the tests actually working, I think I'll leave that up to you, the above diff is trivially small as it is. :-)

[aequitas]

Sorry for the late response, was a little busy yesterday. Good to hear your problem is solved.

The test suite just tests against expectations derived from the concourse spec. They may need to be modified as they are making wrong assumptions now (like what an empty check should return). The fun errors you get are indeed probably caused by incorrect version pinning. I think 3.6 dropped asyncio module for build-ins.

I understand your desire to reduce image size, it is noble, it's one of the things I strive for to. But simply removing the testsuite and calling it a solution is a little harsh. The test suite is to guarantee changes to this resource don't cause it to break for other people (including yourself). I'm curious what you intend to use concourse for if testing is not one of your core principles?

[OvermindDL1]

I think 3.6 dropped asyncio module for build-ins.

I didn't look too much in to it but I think I recall hearing that python 3.6 or 3.7 or so made async a keyword, which broke a few libraries that needed updating.

I understand your desire to reduce image size, it is noble, it's one of the things I strive for to. But simply removing the testsuite and calling it a solution is a little harsh. The test suite is to guarantee changes to this resource don't cause it to break for other people (including yourself). I'm curious what you intend to use concourse for if testing is not one of your core principles?

Actually your Dockerfile.tdd could use the image just fine, the main thing is that for the main release image the alpine version of the official python image was <10% of the debian-based image size. I think it's perfectly fine to test in a larger image, but deployment should be as small as possible.

I'm curious what you intend to use concourse for if testing is not one of your core principles?

Actually I tend to use travis, gitlab-ci, and a hoard of bots, I'm setting up and hosting concourse for someone else (and taking the opportunity to learn it). :-)

And testing is quite important, but I have a slight dislike against artificial test-suites as they rather often don't match what actually needs to be done, so I prefer to setup tests that test against official API's (sandboxed or so of course). That combined with strongly statically typed languages (I'm more of a C++(withOwnershipPatterns)/OCaml/Rust person) means I never really have many issues. :-)

Thanks much!!

[aequitas]

This is one of my earlier resource builds so it probably carries along some misconceptions I had back then about docker. But I optimized by running the tests (and cleanup https://github.com/aequitas/concourse-http-api-resource/blob/master/test/test.sh) in one 'layer' https://github.com/aequitas/concourse-http-api-resource/blob/master/Dockerfile#L11 Which should result in the files not ending up in the final image as they are removed before the layer is created. Maybe I'm missing out some cache somewhere in the cleanup?

Since I want to keep these resources simple I opted to do the test at the end of the build since then a simple docker automated build would suffice. This way you'll never end up with an image that does not confirm to the specs that are specified in the test.

Of course it would be best to test this automatically in a complete concourse setup. But good luck doing that with Travis of docker hub after all this is a just simple open source community resource. I have other projects to live out my CI fantasies :)

Good luck on your quest with concourse, keep a lookout for the developments around spatial resource flows, that would make concourse really take off.

[OvermindDL1]

Maybe I'm missing out some cache somewhere in the cleanup?

I only removed the tests so I could get it to docker build at all, as with them it was crashing every time I tried. The size reduction was on the FROM line change, it was changed to using python on alpine python:3.4.9-alpine3.8 instead of python on debian python. Alpine is significantly smaller (and often faster as well) than debian or other such based images.

Of course it would be best to test this automatically in a complete concourse setup. But good luck doing that with Travis of docker hub after all this is a just simple open source community resource. I have other projects to live out my CI fantasies :)

Yeah that would be better suited for a docker runner like on gitlab-ci or so (or concourse) as you could spin up a concourse docker image just for testing. Travis is pretty limited, but gitlab-ci (which you can use with github, just have to set the hook) is a lot more powerful, especially if you let it run a worker for your project on your own computer or so. :-)

Good luck on your quest with concourse, keep a lookout for the developments around spatial resource flows, that would make concourse really take off.

Yeah I've looked at that in the issue tracker, it definitely looks like something concourse needs! I'm liking concourse so far, my only niggles with it to date is I've needed to make a dozen new docker images that I then have to publish (I reaaaally hate polluting the docker hub registry with useless specialized images, I'm half tempted to spin up my own docker registery at this point), but I imagine that will be reduced quite a lot once more generic resources get created and published, and the webgui is just sooo sloooooow, I've not looked in to it yet in detail but it 'feels' like the web gui is polling instead of using websockets or server push or so, and it's javascript is awfully heavy considering the same interface with the same features could use such substantially less javascript and thus use less JIT memory to boot. I expect all these issues to get ironed out 'in time', but it definitely has a few rough edges thus far (better than jenkins by far though!).

[OvermindDL1]

Just checked the web gui and oh hey, it is polling! Why on earth?! o.O That seems needlessly heavy... And it looks like they are using Elm for the rendering, they should have gone with bucklescript instead of they wanted to go that route, it makes faster and smaller code while having more features and being more safe to use (bucklescript-tea is a library that mimics Elm in bucklescript too if they want to go that way). Honestly the whole interface is simple enough that just javascript could have been used without too much issue (though I'm a fan of statically typed languages so bucklescript definitely wins there).

Let's profile this... Oh wow, the GUI is updating multiple times a second, mostly doing nothing but it is causing the browser to re-render, so that's slow, and whenever you click or do anything it drops to <6fps for a half second to 2 seconds. It appears to be generating svg, which on it's own isn't bad, but they aren't grouping or reusing elements properly, so when updates happen it is causing the great majority of the screen to rebuild and/or re-render instead of only relevant sections, this is not at all even remotely efficient... This is almost as battery-and-rendering-inefficient as you can make a webpage short of making a realtime game in it... ^.^;