Is the implicit get after a put necessary?

[pn-santos]

Is the implicit get that concourse does after a put is necessary for this resource to work? or could it be skipped?

I've had a look at the source code and there seems to be no way to use something similar to the get_params: {skip_download: true} of the docker image resource to skip the implicit get step (please correct me if I'm wrong).

In my case, I'm running 4 "checks" concurrently and at first glance, the get that is always executed after I update the checks status (with put) seems unecessary... Would it make sense to check for a "skip" in the in step?

[itsdalmo]

Thanks for submitting and issue! I totally agree with you that we should include a skip_download parameter and perhaps default it to true (since we don't push code with this resource anyway, just comments and/or statuses).

I'll want to verify whether Concourse actually does a get after each put first, since it might be smart enough to reuse the container from the initial get when the subsequent put's don't emit new versions. Will report back here!

[pn-santos]

If it helps this is the logs I see for implicit gets:

Initialized empty Git repository in /tmp/build/get/.git/
Switched to a new branch '8697f5223bb5fa865736aece316748192a7c22fe'
Updating 8697f52..95a1afa
Fast-forward

[itsdalmo]

I think we'll run into a problem if we try to skip cloning the repo on the implicit get after a put; the get will still run and if we don't fetch the pull request, the pull-request volume will be empty when it's time to run our tests:

jobs:
- name: test
  plan:
  - get: pull-request
    trigger: true
    version: every
  - put: pull-request
    params:
      path: pull-request 
      status: pending 
    get_params: { skip_download: true } <--- pull-request volume will be empty!
  - task: test
    input_mapping: { source: pull-request } <--- Oh no!

I've asked in the Concourse discord to validate whether or not this is how it works 😅

[pn-santos]

ups sorry closed by accident eheh, alright I'll wait for the feedback from the concourse team

[vito]

@itsdalmo Yeah, if you implement skip_download it'll break that example. I've had to "alias" the step to prevent that, something like:

put: update-status
resource: pull-request
params: {...}
get_params: {skip_download: true}

This will still have the implicit get but its (empty) bits will be named update-status rather than clobbering the original pull-request bits.

Which is super janky, but well, all PR workflows will be until we finish spaces support (and perhaps a notifications API for communicating commit status would be nice). :)

[itsdalmo]

Thanks @vito! Looking forward to resources v2, but what you are suggesting seems clean enough for now 😄 What do you think @pn-santos ?

[pn-santos]

So if I understand correctly, I would need to "alias" the puts for which I want to skip the implicit get and I would need to add the get_params.

If that's so seems like an acceptable compromise.

I was actually already thinking of aliasing them anyway. I'm using this resource to set 5 separate checks on github and having parallel puts (3 for each check, 1 pending, 1 for failure and another for success) makes for a very confusing build log since it will cause the pull request resource name to be repeated all over the place. Aliasing makes it easier to distinguish them.

[itsdalmo]

@pn-santos - I've merged the PR to add skip_download as a parameter to get and it should be available under the dev tag any minute: https://hub.docker.com/r/teliaoss/github-pr-resource/tags/

Would you mind taking it for a spin and seeing if it works? I'm travelling today so unfortunately cannot test it in the wild myself 😄

[pn-santos]

Sure, I'll report back the results, thx for the quick turn around 👍

[pn-santos]

Everything seems to work ok, only had 1 minor issue, when using:

get_params: { skip_download: true }

I got this:

2018/09/19 20:32:23 failed to unmarshal request: json: cannot unmarshal bool into Go struct field GetParameters.skip_download of type string

I guess SkipDownload type should be bool instead of string?

When I switched to:

get_params: { skip_download: "true" }

then everything worked 💯

[itsdalmo]

Thanks for testing this @pn-santos - #37 made skip_download into a boolean and has been released under the v0.6.0 (and latest) tag.