Skip to content
This repository has been archived by the owner on Jul 12, 2022. It is now read-only.

Gitea Integration #711

Merged
merged 22 commits into from
Apr 18, 2019
Merged

Gitea Integration #711

merged 22 commits into from
Apr 18, 2019

Conversation

sharpSteff
Copy link
Contributor

@sharpSteff sharpSteff commented Apr 12, 2019

Started to work on Gitea(issue #710 ) support for NuKeeper based on https://try.gitea.io/api/swagger# and https://docs.gitea.io/en-us/

What's already there:

Implementation of settings reader + some tiny unit tests
Basic implementation of gitea rest client and token authentication including:

  • GetCurrentUser
  • GetRepository
  • GetOrganizations (only for admins)
  • GetRepositoryBranch
  • ForkRepository
  • OpenPullRequest

At the end a lot of code in the restclient is similar to GitHub/Gitlab. Maybe a common base class would remove this redundant code.

TODO:

  • GiteaPlattform.GetRepositoriesForOrganisation (and REST)
  • GiteaPlattform.Search (and REST)
  • Enhance GiteaRepositoryDiscovery.GetRepositories for ServerScope.Global and ServerScope.Organisation
  • GiteaForkFinder implement other functionality than ForkMode.SingleRepositoryOnly

Unit tests

  • Use real repository for unit tests (??)

@sharpSteff
Copy link
Contributor Author

pullrequest_nukeeper
gitea_pull

Here a some example screens, after forking nukeeper to a local hosted gitea and running nukeeper against it :)

@AnthonySteele
Copy link
Member

This is great!

Agreed that soon we should focus on common code across collaboration platforms (perhaps a CollaborationPlatform.Abstractions project? ) And on getting them better tested.

@MarcBruins
Copy link
Member

Could you also add some docs on how to use it? Something like this https://github.com/NuKeeperDotNet/NuKeeper/pull/720/files#diff-32cb44efbeb7ab3bf9cf2edef28be295R69 Its just to show other people how it works. There is a gitea.md file available here: https://github.com/NuKeeperDotNet/NuKeeper/blob/master/site/content/platform/gitea.md. If you pull master you can edit that file and add it to this PR.

@MarcBruins
Copy link
Member

The bitbucket explanation is probably a better startpoint: https://github.com/NuKeeperDotNet/NuKeeper/pull/724/files#diff-dce3466c4d63c7bf1684bbfa5a2e235f

@sharpSteff
Copy link
Contributor Author

sharpSteff commented Apr 14, 2019

@MarcBruins Thanks for the startpoint!

Following points are also finished:

  • Enhance GiteaRepositoryDiscovery.GetRepositories for ServerScope.Global and ServerScope.Organisation
  • GiteaForkFinder implement other functionality than ForkMode.SingleRepositoryOnly

so automatically forking from an existing repository also works from now on

@sharpSteff
Copy link
Contributor Author

@MarcBruins
I recognized ForkMode has no default value in FileSettings.
Is this intended ?

@AnthonySteele
Copy link
Member

The default is (or should be) PreferFork

@sharpSteff
Copy link
Contributor Author

when there is no nukeeper.settings.json-config file (which i has to create manuelly, by the way) or a nukeeper.settings.json-config without ForkMode defined, ForkMode might be null since its explicitly declared as a nullable type in FileSettings

public ForkMode? ForkMode { get; set; }

In CollaborationFactory.ValidateSettings() ForkMode will be checked

if (!Settings.ForkMode.HasValue) { return ValidationResult.Failure("Fork Mode was not set"); }
In result, the operation will be cancelled.

However when a nukeeper.settings.json is available with ForkMode defined everything works as expected

NuKeeper.Gitea/GiteaPlatform.cs Outdated Show resolved Hide resolved
NuKeeper.Gitea/GiteaRestClient.cs Show resolved Hide resolved
client.DefaultRequestHeaders.Accept.Add(
new MediaTypeWithQualityHeaderValue("application/json"));

HttpResponseMessage response = client.GetAsync($"swagger.v1.json").Result;
Copy link
Member

Choose a reason for hiding this comment

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

Please no .Result. This will block the code untill it's complete

Copy link
Contributor Author

Choose a reason for hiding this comment

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

issue here was CanRead() is no Task. So I had to go the sync way
Any recommandations how to solve this ?

Copy link
Member

Choose a reason for hiding this comment

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

So the only way to know if it's a gitea server is to call the swagger api? There is no convention whatsoever that must be used? If not, i think we should have an extra parameter which specifies that it's the desired platform: --platform gitea

Copy link
Member

Choose a reason for hiding this comment

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

I think we discuss this approach earlier on gitter or somewhere else.. I don't want to make this a blocker, would be nice if we could add it but it doesn't have to be in this PR. I could be a new upforgrabs feature.

Copy link
Contributor Author

@sharpSteff sharpSteff Apr 17, 2019

Choose a reason for hiding this comment

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

CanRead - does only provide the repo-uri. Since gitea is most of the time self hosted, the repo uri looks like this
https://localhost:12345/{username}/{projectname}.git

So trying to get the json from https://yourgitea/swagger.v1.json (maybe even trying to validate this json, since there are gitea-version, etc stored) was somehow the safest way.

I think if nukeeper wants to support self hostet github or vsts services, this issue will appear again sooner or later.

a desired platform-parameter would solve this issue however

Copy link
Member

Choose a reason for hiding this comment

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

We have the --platform option already. See https://github.com/NuKeeperDotNet/NuKeeper/wiki/Configuration

As the docs say:

This is typicaly infered from the api url structure, but since this does not always work, it can be specified here if neccessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AnthonySteele so when using --platform, the CanRead(uri) will be skipped

NuKeeper.Gitea/GiteaSettingsReader.cs Show resolved Hide resolved
NuKeeper.Gitea/Model/UserPermissions.cs Outdated Show resolved Hide resolved
@MarcBruins
Copy link
Member

when there is no nukeeper.settings.json-config file (which i has to create manuelly, by the way) or a nukeeper.settings.json-config without ForkMode defined, ForkMode might be null since its explicitly declared as a nullable type in FileSettings

public ForkMode? ForkMode { get; set; }

In CollaborationFactory.ValidateSettings() ForkMode will be checked

if (!Settings.ForkMode.HasValue) { return ValidationResult.Failure("Fork Mode was not set"); }
In result, the operation will be cancelled.

However when a nukeeper.settings.json is available with ForkMode defined everything works as expected

Can you make preferFork the default?

client.DefaultRequestHeaders.Accept.Add(
new MediaTypeWithQualityHeaderValue("application/json"));

HttpResponseMessage response = client.GetAsync($"swagger.v1.json").Result;
Copy link
Member

Choose a reason for hiding this comment

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

So the only way to know if it's a gitea server is to call the swagger api? There is no convention whatsoever that must be used? If not, i think we should have an extra parameter which specifies that it's the desired platform: --platform gitea

client.DefaultRequestHeaders.Accept.Add(
new MediaTypeWithQualityHeaderValue("application/json"));

HttpResponseMessage response = client.GetAsync($"swagger.v1.json").Result;
Copy link
Member

Choose a reason for hiding this comment

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

I think we discuss this approach earlier on gitter or somewhere else.. I don't want to make this a blocker, would be nice if we could add it but it doesn't have to be in this PR. I could be a new upforgrabs feature.

NuKeeper.Gitea/GiteaRestClient.cs Outdated Show resolved Hide resolved
NuKeeper.Gitea/GiteaPlatform.cs Outdated Show resolved Hide resolved
@sharpSteff
Copy link
Contributor Author

Can you make preferFork the default?

I think I'll create an additioanl issue for this

@MarcBruins MarcBruins merged commit 6a6e63c into NuKeeperDotNet:master Apr 18, 2019
@zubivan zubivan mentioned this pull request Apr 24, 2019
2 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants