Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graphqa api changes #107

Merged
merged 3 commits into from
Sep 8, 2021
Merged

Conversation

kirillbilchenko
Copy link
Contributor

@kirillbilchenko kirillbilchenko commented Sep 8, 2021

After today github release the id field is now changing the format, so resource is returning error running command: illegal base64 data at input byte 2
https://docs.github.com/en/graphql/reference/objects#release

Signed-off-by: kbilchenko <kirya7@gmail.com>
@xtremerui
Copy link
Contributor

Hi could you add a link to the github graphql change log please? Thx!

@qvistgaard
Copy link

qvistgaard commented Sep 8, 2021

@xtremerui it's not included in the changelog. the databaseId was added here:

https://docs.github.com/en/graphql/overview/changelog#schema-changes-for-2021-06-07

But if you look at the graphql api documentation the id field is not even mentioned: https://docs.github.com/en/graphql/reference/objects#release

it is wierd though because it is mentioned in the scalars, and described as a base64 encoded string, but if you look at the api it's no longer that.

@kirillbilchenko
Copy link
Contributor Author

@xtremerui I'm not sure how helpful will be this, but github_communtiy there is an answer about the id, and it was working for some time, until today when I was working on this graphql client the databaseId field was not yet available. Github release new enterprise server version yesterday, so most probably prod were also updated the link on release notes https://docs.github.com/en/enterprise-server@3.1/admin/release-notes

@kirillbilchenko
Copy link
Contributor Author

@xtremerui one more small note, we already build the image of updated resource and replaced the build in github-release resource inside worker, and now it's working fine on our side.

@xtremerui
Copy link
Contributor

I am wondering the circumstance of using databaseId or ID.

The changes here shows it should use databaseId when it is available, if not it will try to fall back to ID. So I am looking for the offical doc about the relationship between these two fields.

Also, the way we decodes the ID field hasn't changed. So I am a bit confused on the id field is now changing the format part.

@kirillbilchenko
Copy link
Contributor Author

I am wondering the circumstance of using databaseId or ID.

The changes here shows it should use databaseId when it is available, if not it will try to fall back to ID. So I am looking for the offical doc about the relationship between these two fields.

Also, the way we decodes the ID field hasn't changed. So I am a bit confused on the id field is now changing the format part.

This one I left so self-hosted installation, not sure when the databaseId field was introduced, so before today old id was working just fine, I kept it as a fallback in case of someone is using the old version where there is no datatabaseId field.

@xtremerui xtremerui merged commit de55db2 into concourse:master Sep 8, 2021
@techgaun
Copy link
Member

techgaun commented Sep 8, 2021

any chance we are going to get this pushed out to docker hub? I only saw dev and dev-ubuntu tag from today.

@xtremerui
Copy link
Contributor

@techgaun just cut a new release. it should be up now.

@kirillbilchenko kirillbilchenko deleted the graphqa_api_changes branch September 9, 2021 05:05
xtremerui pushed a commit that referenced this pull request Sep 13, 2021
we need the changes in #107 for 6.7 branch as well

Signed-off-by: Rui Yang <ruiya@vmware.com>
@adamdossa
Copy link

Apologies for the noob question - our concourse pipeline is failing with:

check failed
run check step: run check step: check: resource script '/opt/resource/check []' failed: exit status 1

stderr:
�[31merror running command: illegal base64 data at input byte 2
�[0m

I can see a fix has been pushed for the github-release resource, but wondering how to force our concourse instance to pick up the new version?

@xtremerui
Copy link
Contributor

@adamdossa add the following custom resource type to your resource_types: section so it can overwrite the default one that is failing

- name: github-release
  type: registry-image
  source: {repository: concourse/github-release-resource}

joyvuu-dave added a commit to pivotal-cf/telemetry-release that referenced this pull request Sep 17, 2021
ljfranklin referenced this pull request in ljfranklin/terraform-resource Sep 20, 2021
katstevens added a commit to alphagov/pay-ci that referenced this pull request Sep 30, 2021
Github recently changed their release formatting behind the scenes (see concourse/github-release-resource#107). The fix has been pushed for the Concourse github-release-resource, but this is not available in the version of Concourse that BigConcourse is currently running (v7.2.0). Given it may be 'a while' before BigConcourse reaches 7.5.0+, we can instead pin to the version of the resource repo that contains the fix, and specify this as a resource_type.

The naming of the resource type is a bit clumsy but I want it to be clear that this is a version of the resource, not the bleeding edge of the repo. Suggestions for a better name welcome.
katstevens added a commit to alphagov/pay-ci that referenced this pull request Sep 30, 2021
Github recently changed their release formatting behind the scenes (see concourse/github-release-resource#107). The fix has been pushed for the Concourse github-release-resource, but this is not available in the version of Concourse that BigConcourse is currently running (v7.2.0). Given it may be 'a while' before BigConcourse reaches 7.5.0+, we can instead pin to the version of the resource repo that contains the fix, and specify this as a resource_type.

The naming of the resource type is a bit clumsy but I want it to be clear that this is a version of the resource, not the bleeding edge of the repo. Suggestions for a better name welcome.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants