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

Update gitlab API to version v4 #2068 #2161

Merged
merged 1 commit into from Sep 9, 2017

Conversation

vanadium23
Copy link
Contributor

Hello, fellows.

I've moved drone gitlab client from api/v3 to api/v4, also slightly change unit test so they've been passing, but imo we need to refactor them in the way other clients do.
I've tested it against local gitlab and gitlab.com and both works, except commit status is not changing, but it's broken from gitlab API.

@CLAassistant
Copy link

CLAassistant commented Aug 11, 2017

CLA assistant check
All committers have signed the CLA.

@kzaitsev
Copy link
Contributor

👍 Good job

@bradrydzewski
Copy link
Contributor

thanks for this! so it sounds like status API is broken on the gitlab end? can you provide some more details so we can document as a known issue and maybe link to the gitlab issue?

@vanadium23
Copy link
Contributor Author

@bradrydzewski don't know what is wrong. According to gitlab docs, the implementation is correct. Also I've rebased code onto master. I can deal with commit status in separate issue, if you want (:

@bradrydzewski
Copy link
Contributor

bradrydzewski commented Aug 28, 2017

@vanadium23 maybe check to make sure you have DRONE_HOST configured properly? If not properly configured it can impact the ability to create statuses.

for example DRONE_HOST=https://beta.drone.io

@amq
Copy link

amq commented Aug 28, 2017

Adding the scheme to DRONE_HOST solved the status updates for me.

@vanadium23
Copy link
Contributor Author

@bradrydzewski, yes, problem was in the scheme, because I set it as localhost (:

@@ -1,17 +1,16 @@
package client

import (
b64 "encoding/base64"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove alias and use base64 package name below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@thomasf
Copy link
Contributor

thomasf commented Aug 31, 2017

this issue closes #2068 (don't know why github doesnt link these issues when the issue # is in the title)

btw. Gitlab decided to keep the v3 api around a bit longerbecause they had too many v3 api requests to their site.. So latest gitlab still works with drone which is good

@vanadium23
Copy link
Contributor Author

@bradrydzewski everything is fine?

@bradrydzewski
Copy link
Contributor

ok, sorry for the delays. thanks again for this! I plan to snapshot rc.4 on monday for those waiting for a tagged release.

@bradrydzewski bradrydzewski merged commit fcb493b into harness:master Sep 9, 2017
@Spedge
Copy link

Spedge commented Sep 14, 2017

So we've been working on upgrading our Drone version to 0.8 - installing and delivering on rc3. rc4 is released and all of a sudden doesn't work with our supported version of GitLab? So to use Drone, we now need to do a major version upgrade of GitLab?

As @thomasf suggests - v3 is still around. By migrating to v4 and not supporting any backward compatibility, you are really making this difficult for us.

@thomasf
Copy link
Contributor

thomasf commented Sep 14, 2017

I don't think it's unfair to require the v4 API, the v3 API will not be
maintained anymore (whatever that means). Gitlab announced that v3 was going
away many months (~a year?) before it happened or something and v3/v4 has been
working in parallell in Gitlab for a long time giving people time to migrate.

@Spedge
Copy link

Spedge commented Sep 14, 2017

If we wrote some code to allow us to add a flag to the startup in order to use the v3 version, would you accept it?

@bradrydzewski
Copy link
Contributor

bradrydzewski commented Sep 14, 2017

@Spedge no worries. I just merged #2215

I ended up just copying the old gitlab code into a gitlab3 folder so that we have old and new code in sibling packages. This way I didn't have to make any major code changes in drone.

remote/gitlab
+remote/gitlab3

I then added an environment variable that you can use to switch:

DRONE_GITLAB_V3_API=true

This change is available in the drone/drone:latest image. Would it be possible to update your drone server image and verify this change by end of day tomorrow? I only ask because I am planning on snapshotting a final release on Monday and would like to make sure there are no critical issues.

@gjtempleton
Copy link

I've just tried out the :latest tag with DRONE_GITLAB_V3_API=true set and successfully triggered builds and had the status reported back to Gitlab successfully. Thanks for the swift response.

@bradrydzewski
Copy link
Contributor

Thanks Guy for the quick reply, happy to help!

@Spedge
Copy link

Spedge commented Sep 15, 2017

Brilliant, thanks Brad - really, really appreciate the quick turnaround on this.

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.

None yet

8 participants