GraphQL API proof of concept: merge request widget endpoint

changed title from GraphQL API proof of concept and frontend implementation: merge request widget endpoint to GraphQL API proof of concept: merge request widget endpoint

/cc @DouweM

I started playing around with GraphQL last weekend out of sheer interest. I was able to get a simple implementation working with GitLab (albeit, no authorization, etc). Happy to share what I learned. GraphQL is fun and interesting.

changed milestone to %10.0

Pushed back to %"9.6"

changed the description

@dblessing that sounds awesome. I'll be working on this a little in %9.5 but it's mostly for %9.6 now. Do you have a branch with your implementation that you could share?

added Deliverable label

added frontend and removed Deliverable labels

added Deliverable label

Hey guys! Can we not start with the MR widget? 😊 Let's start with the sidebar or something like that. The MR widget has around 21 states.

🚗 📘 🎱

@jschatz1 We specifically need a component that uses data from multiple related records, so that we can actually start using GraphQL for its graph functionality, instead of using it just like we would a single-resource REST API.

changed the description

mentioned in issue #22753 (closed)

mentioned in merge request !13601 (closed)

The bones of an implementation are taking shape in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13601

Half a day's work allows us to query both projects and merge requests, including authentication and authorization. There are TODOs scattered throughout, and we're not publishing all the attributes the grape API exposes yet, but I'm impressed with how simple it's been so far.

Mandatory screenshot:

We need a project's full path to get merge requests. We can either get all MRs or pick a single one by IID. We could have a top level merge_request(id: x) endpoint, or add finding projects by id, but I'd like to see how far we can get without exposing ids at all!

I'm not sure at this point how much of the Grape API we can re-use. In particular, we need type data not present in the grape entities. However, much of the heavy lifting is in writing resolvers (for querying) and mutators (for state changes). It's possible that we can implement the grape endpoints in terms of graphql calls to keep us from needing two independent APIs.

@timzallmann do you know which graphql client library you're going to be using for this? I ask as it particularly affects server-side pagination support.

Relay suggests a cursor-based approach: https://facebook.github.io/relay/graphql/connections.htm
vue-apollo uses page, pageSize and hasMore: https://github.com/Akryum/vue-apollo#pagination-with-fetchmore
vuex-apollo example: https://github.com/PierBover/vuex-apollo-example-project - I assume this uses the same pagination scheme as vue-apollo

@nick.thomas Great progress ! So far I looked mainly at vue-apollo, looks good so far but will discuss it also with the frontend architecture group, for the POC i would say we go most probably with Apollo so I would say lets go for vue-apollo approach for paging.

Rightyo. I'll note that the vuex-apollo author wrote:

I wrote this since everyone seems to be using vue-apollo which encourages to put the GraphQL queries in Vue componentes. This is an antipattern and should never be done. The whole point of using Vuex is to centralize state and decouple your data layer code from your components.

I have no idea whether they're right or not, but since they use the same approach to pagination, it's all the same to me 👍

@nick.thomas He is totally right, we already use Vuex and i would also see that completely handled in the store and not in any component

Great to know 👍

@timzallmann do you have any preferences on how you would query the merge request? Right now it's always wrapped by a project toplevel, but we could have a merge request toplevel that takes [project_full_path, mr_iid] to return a single MR?

Current options:

(although queries should be static and use variables, of course, per http://graphql.org/learn/queries/#variables)

@nick.thomas having top-level merge_requests might be useful, e.g. to query all MRs assigned to me (I recently added this to API v4 in gitlab-org/gitlab-ce!13060).

One question though, with that top-level merge_requests, would it be possible to query for multiple MRs (without using aliases)?

Now that being said, I feel we should keep the first iteration of GraphQL as simple as possible. And IMHO I find querying by iid makes most sense when it's wrapped in a project.

@to1ne yep, for this first iteration we're just focused on the MR widget, which only needs access to a single MR in a project. Search-like capabilities aren't going in yet.

I've got the project { merge_requests { ... } } endpoint mostly as a testbed for GraphQL::Batch at the moment, I'll probably remove even that once I've got user support (since that'll make use of batching too).

Hey so I know that GraphQL is awesome sauce, but have we talked about the custom Facebook License that is tied to the GraphQL ~~spec~~ frontend lib?

The apache foundation has recently said that "..the Facebook BSD+patents license has been moved to Category X (https://www.apache.org/legal/resolved#category-x).."

This pretty much means the apache foundation has put the FB+PL License in the banned licenses bucket.

That quote can be found by reading this thread: https://issues.apache.org/jira/browse/LEGAL-303

Here is a hackernews thread about it: https://news.ycombinator.com/item?id=14779881

We should have a healthy discussion about putting a license like this in our codebase 😄

@selfup thanks for raising this, it's not something I'd considered.

Is this the spec we're talking about: https://github.com/facebook/graphql ?

The HN thread seems to be all about content in a PATENTS file, which doesn't appear.

There is a license present here: https://github.com/facebook/graphql/blob/master/spec/GraphQL.md which seems, to my eyes, to be a standard 3-clause BSD?

The implementation we're using has a standard MIT license: https://github.com/rmosolgo/graphql-ruby/blob/master/MIT-LICENSE

I also don't see any PATENTS files in that repo.

Are you certain "the GraphQL spec" is affected by this controversy?

Ahh you might be correct on that front. Still odd that there is a BSD-3 clause for a spec 🤔 Thanks for the catch!

On another note, if our frontend ever switches to Flow we will have to deal with a PATENTS file: https://github.com/facebook/flow, but it would only be a dev dependency and not show up in our source code so we should be ok.

Seems they are using that license only in certain places. Read below for more info

@nick.thomas Wait, what about this?

https://github.com/graphql/graphql-js

Ok seems that vue-apollo uses: https://github.com/apollographql/apollo-client

By looking in: https://github.com/Akryum/vue-apollo/blob/master/package.json

And the ruby gem we are using is MIT as you said. 👍

~~I think we should be ok!~~

After Looking A Bit More

https://github.com/apollographql/apollo-client/blob/master/package.json#L76

It seems that apollo-client uses the PATENTS file bound graphql lib 😭

Unclear how the apollo-client lib is MIT if it uses graphql as a dependency though 🤔

@ernstvn am I right in thinking that we come to you with license quandries?

I know @timzallmann was intending to use vue-apollo; does this mean we cannot?

Do we have an alternative JS implementation we can go to if this is indeed the case?

@nick.thomas Alas... you are not right in thinking that I tackle license quandries. From what I recall, @connorshea built a sweet tool to trace licenses of dependencies, and @pmachle tends to have final say on what we accept or not. Paul, is this something that we should ping @jhurewitz on from now on?

mentioned in issue gitlab-com/organization#117

I've spotted a section in the docs relating to getting new licenses approved: https://docs.gitlab.com/ce/development/licensing.html#requesting-approval-for-licenses

I've followed it for two license queries this issue throws up:

However, @gl-legal doesn't seem to be a working alias.

@nick.thomas The address is legal@gitlab.com.

mentioned in issue #36883 (closed)

added In dev label

assigned to @timzallmann

@timzallmann I've gone through https://gitlab.com/gitlab-org/gitlab-ee/blob/master/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js to try to work out exactly what data you're going to need these endpoints to return.

I think I've arrived at:

(this comment is WIP, I'll remove this line once I've finished updating it)

current_user {
}

merge_request(full_path: "gitlab-org/gitlab-ce", iid: 1) {
  title

  state # for 'is_open', etc

  target_branch
  source_branch

  # To check: are these all returned by the API? 
  target_branch_path
  target_branch_tree_path
  conflict_resolution_path
  cancel_auto_merge_path
  remove_wip_path
  merge_path
  status_path
  email_patches_path
  plain_diff_path
  new_blob_path
  create_issue_to_resolve_discussions_path
  merge_check_path
  merge_actions_content_path

  source_branch_removed
  should_remove_source_branch

  only_allow_merge_if_pipeline_succeeds
  merge_when_pipeline_succeeds
  is_removing_source_branch
  has_mergeable_discussions_state
  can_remove_source_branch
  can_merge
  can_create_issue
  can_cancel_automatic_merge
  has_sha_changed
  can_be_merged
  merge_ongoing

  can_cherry_pick_in_current_mr
  can_revert_in_current_mr
  cherry_pick_in_fork_path
  revert_in_fork_path

  ci_environments_status_path
  has_ci
  ci_status
  is_pipeline_failed # secondary data
  # pipeline_detailed_status # pipeline { details { status } }
  # is_pipeline_active # pipeline { active }
  # is_pipeline_blocked # pipeline { details { group } }
  # ci_status_favicon_path # pipeline { details { favicon_path } }

  merge_status

  merge_commit_message
  merge_commit_message_with_description

  commits_count
  diverged_commits_count

  updated_at
  merged_at
  closed_at

  merged_by { # User
    id
    name
    username
    web_url
    avatar_url
  }

  closed_by { # User
    name
    username
    web_url
    avatar_url
  }

  set_to_mwps_by { # User
    name
    username
    web_url
    avatar_url
  }
  
  # Most recent pipeline ?
  pipeline {
    active
    details {
      status
      group

      favicon_path
    }

  }

  # ?
  deployments {

  }

  # ?
  issues_links {
  }
}

I'm missing quite a few of these - particularly pipeline, deployment and user - so I'll concentrate on those next.

Even if we don't get the frontend done in time for %10.0, we can merge the backend, hidden behind a feature flag. Ideally, we'd have both clients available too, and we'd use the same feature flag to decide which to serve.

removed assignee

changed milestone to %Backlog

removed In dev label

This is being pushed back until the legal questions can be answered. Follow https://gitlab.com/gitlab-com/organization/issues/117 and https://gitlab.com/gitlab-com/organization/issues/116 for updates.

There's also emerging questions around the patenting of the GraphQL spec, and what that means for graphql-ruby - e.g. https://medium.com/@dwalsh.sdlr/using-graphql-why-facebook-now-owns-you-3182751028c9

On the technical side of things, the findings in !13601 (closed) were all positive - constructing a full graphql API for GitLab seems completely reasonable. I didn't get around to looking at:

Mutations (but these are just RPCs so shouldn't be troublesome)
EE extensions to graphql types (I assumed this would be easy but the way the classes in app/graphql/types are defined has given me second thoughts).

mentioned in issue #32046 (moved)

@nick.thomas Another alternative could be using Falcor

Repo: https://github.com/Netflix/falcor

Website: http://netflix.github.io/falcor/

added In dev label

We're going to merge a proof-of-concept graphql backend for %10.0 but the MR widget won't make use of it.

Falcor is new to me; I don't see an obvious candidate for the backend.

Should we bring in @jhurewitz to check the licenses here?

@jschatz1 we already did. Facebook BSD+PATENTS is a no. The only code licensed with this that is relevant to this MR is client-side. Unlicense (used by promise.rb, used by graphql-batch, used with graphql-ruby) is a yes.

The GraphQL spec itself is just BSD-licensed with no scary PATENTS file. graphql-ruby is just MIT-licensed with no scary PATENTS file, and no dependencies that have a scary PATENTS file.

At least one person on the internet has brought attention to the fact that FB are registering patents for graphql, per https://medium.com/@dwalsh.sdlr/using-graphql-why-facebook-now-owns-you-3182751028c9 . This doesn't seem to be enough to stop us from merging support for GraphQL.

As I have had 1 very scary and horrible experience with patents in the past, and I am sure @jhurewitz can talk to this. I can't imagine supporting something that a huge company is thinking of patenting. Why would they patent it... There are multiple reasons, but the definition itself is scary enough to make you stop and think, should we be going into this territory?

A grant made by a government that confers upon the creator of an invention the sole right to make, use, and sell that invention for a set period of time.

Sole...

Doesn't seem like a give back to the community sort of thing. Seems like a give some to the community then charge money down the road once the whole community is fully invested.

Falcor on the other hand is Netflix, which is Apache 2.0, which I think we are good with. And they haven't done anything scary like this.

@nick.thomas just confirmed with @jhurewitz: No GraphQL period.

mentioned in merge request !14060 (merged)

For the same reasons with regard to Facebook's BSD and Patent License, the GraphSQL patent is well in process and if granted, we risk the same issue with the licensing creating conflicts.

removed In dev label

@jhurewitz You imagine that if it's granted, if we want to use any GraphQL implementation, even those not built by Facebook, Facebook will require us to agree to something we don't want to?

@DouweM at the moment, the concern is only with regard to components licensed under the Facebook BSD and patent license.

@jhurewitz But graphql-ruby isn't actually licensed under BSD+PATENTS, it's MIT: https://github.com/rmosolgo/graphql-ruby/blob/master/MIT-LICENSE

@DouweM I was not asked about GraphQL Ruby. What was brought to my attention was GraphSQL which is licensed under the Facebook license. I believe there might be a typo above in @jschatz1 comment. The determination was no GraphSQL, which is currently in patent applications by Facebook.

Very unofficial source claiming FB might relicense GraphQL as technology: https://medium.com/@dwalsh.sdlr/hi-ezequiel-6855beab08cb

That might unblock us, some day.

Hi, I'm one of the GraphQL co-creators at Facebook and was referred to this thread. I was made aware of the legal problems by the same medium post referred here. The author of the article opened an issue on the GraphQL spec to discuss as well.

I'd love to better understand the concerns of the Gitlab community with respect to this issue. While I can't make any promises, I'd like to make sure I'm representing concerns correctly as I discuss with Facebook's legal council.

The linked Medium article makes the case that the GraphQL spec being licensed as BSD is insufficient and summarizes with a claim that "Facebook should immediately include a patent grant into the GraphQL spec", suggesting an ideal outcome being a BSD+Patents license. However I read above that this suggested conclusion may not be helpful for Gitlab.

@leebyron Thanks for commenting here. @jhurewitz do you know if a BSD license without Patents is fine with us? The article Lee mentioned is https://medium.com/@dwalsh.sdlr/using-graphql-why-facebook-now-owns-you-3182751028c9

@sytses There is no issue with BSD license without Patents. However, GraphQL patent is pending, and it is unknown whether the Patent license will be added to the component once the patent is approved. The issue with the Patent license is that it is over-reaching and restrictive of the rights of the user by prohibiting any infringement actions against Facebook and automatically terminating rights to use the software. This is not open source community friendly, and the Apache Foundation has rejected it use. If we were to allow this license, it could lead to potential future conflicts with software licensed under Apache. Also, we could be impairing the future rights of our customers. Essentially, this is not really an open source product based on the implications of the license. While there is no payment of cash, payment is in the form of giving up future rights.

~~@jhurewitz you mean @sytses...~~ Fixed... Thanks!

@jhurewitz isn't it true that when you have a patent on software it does not really matter what the license is because it could be changed at anytime? Because patents give one the sole right to that invention they are free to do with it as they please? The problem becomes if we are "all in", and completely invested in GraphQL and the license changes we can be in trouble. As far as I understand, you can change the software license if you own the software, and a patent implies ownership, right? I asking, not stating. 😄

And you say

There is no payment of cash

If we invest money in developing software around GraphQL and we suddenly wind up in a different situation then intended, then that would be, in a way, a loss of cash.

@jschatz1 yes the license can be changed at any time, and given Facebook's other use of the BSD plus patent license, it seems highly likely this would also be changed to include the patent license. And yes if we were to allow this component to be included in our product under the BSD now, we would be out development hours if we needed to remove it down the road, should the patent license be added.

@jhurewitz should we add a note to the Licensing doc that we should avoid Facebook libraries for this reason?

@connorshea it is already on the "Unacceptable Licenses" list. Are you looking for something in addition to this?

If referenced this issue in https://github.com/facebook/graphql/issues/351#issuecomment-330574513 which I linked in a tweet https://twitter.com/sytses/status/910162531750051841

Quoted in https://www.theregister.co.uk/2017/09/20/gitlab_suspends_graphql_project_over_facebook_license_terms/

Note the similar position by WordPress with respect to this license. https://techcrunch.com/2017/09/15/wordpress-to-ditch-react-library-over-facebook-patent-clause-risk/

@jschatz1 @stanhu @nick.thomas @jhurewitz @nick.thomas

Did you had a look on the Apollo Stack from Meteor before deciding to completely cancel the GraphQL plan? It comes with a MIT license and it doesn't use any Facebook licenses as far as I can tell. Apollo is Meteor stack agnostic so no Meteor tooling is required. I've recommended Apollo already here https://gitlab.com/gitlab-org/gitlab-ce/issues/22753#note_17693141 once. The Apollo stack is very stable and mature and supports already GQL features such as subscriptions. Not sure if that is even yet supported by the GQL implementation from Facebook. As you're using Vue in your frontend Apollo would be actually a perfect fit as there is a Vue Apollo integration available.

I can only recommend not giving yet up on the GraphQL plan as it makes development so much easier and intuitive as compared to REST. Looking forward to the GraphQL based merged request widget :)

Edit: Sorry Apollo doesn't avoid the GraphQL spec patent issue :( Wondering what the Apollo guys are saying to that issue.

@subesokun The issue isn't the license of the implementation library. The problem is that any library that implements the GraphQL spec could be in violation of the patent that Facebook will shortly have on the spec.

@dblessing Yes sorry, noticed that as well after writing my comment above. I've opened an issue on Apollo to ask for their opinion https://github.com/apollographql/apollo/issues/106

@subesokun in addition to the patent issues with the spec, we also encountered a problem with vue-apollo. It depends on facebookm's JS graphql client, which uses facebook's BSD+PATENTS license: https://gitlab.com/gitlab-org/gitlab-ce/issues/34754#note_38092091

graphql-ruby, which is the backend library we were using, is plain MIT but the graphql patent issue overshadows it so is a no-go for GitLab.

@nick.thomas Mh seems like your referenced comment https://gitlab.com/gitlab-org/gitlab-ce/issues/34754#note_38092091 is not up-to-date anymore. I can't see any dependency on Facebook's graphql library, see package.json. Also vue-apollo has no dependency on Facebook libraries, see package.json 👍

Hopefully the GQL patent issue gets solved soon so that Apollo is save to use.

They are relicensing React under MIT https://code.facebook.com/posts/300798627056246

That is great. Hopefully they will make an announcement about the fate of GraphQL as well.

@sytses @jhurewitz even with the GraphQL being MIT, if they have a patent doesn't that make all this null and void because they can change the license whenever they want?

@jschatz1 patent or no patent, anyone can change their license whenever they want. It is a matter of whether you trust that they are not going to change it in a way that is detrimental, after spending the resources incorporating the component into the product, causing you to spend further resources replacing the component due to the impact on your own licensing strategy.

@jhurewitz So any potential license change does not really change whether we would adopt GraphQL or not?

@jschatz1 without further insight into how GraphQL will be licensed in light of patent approvals, we still assume it will be BSD plus patent, which is an unapproved license. If it is announced that it will be licensed without the patent license, or some other license on the approved list, then we could use it, as it is the patent portion of the license which landed it on the unapproved license list.

@jhurewitz - https://medium.com/@leeb/relicensing-the-graphql-specification-e7d07a52301b

Does this mean we can proceed with this?

To summarize, AFAICT:

GraphQL code released by facebook is now MIT-licensed with no dodgy patent grant
The GraphQL spec licensed as http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0 which covers patents and copyright

@nick.thomas I will need to look at the OWFa 1.0 license before I can respond.

Happy to see this thread already updated with links to the relicense announcement. I'm working today to get versions of our core libraries out with MIT license as well.

I really hope to see Gitlab be able to resurrect this effort.

@leebyron Thank you for your hard work. We are working equally hard to get the new license approved. You have made many developers at GitLab happy, as they are eager to use GraphQL.

moved to gitlab#10896

Hey! 👋

GitLab is moving all development for both GitLab Community Edition and Enterprise Edition into a single codebase. The current gitlab-ce repository will become a read-only mirror, without any proprietary code. All development is moved to the current gitlab-ee repository, which we will rename to just gitlab in the coming weeks. As part of this migration, issues will be moved to the current gitlab-ee project.

If you have any questions about all of this, please ask them in our dedicated FAQ issue.

https://gitlab.com/gitlab-org/gitlab-ee/issues/13855

You can also find more information here:

https://gitlab.com/gitlab-org/gitlab-ee/issues/13304
https://about.gitlab.com/2019/08/23/a-single-codebase-for-gitlab-community-and-enterprise-edition/

Frequently Asked Questions

For an up to date list of questions and answers, please take a look at https://gitlab.com/gitlab-org/gitlab-ee/issues/13855

What will we do with the repository names?

https://gitlab.com/gitlab-org/gitlab-ee/ will become https://gitlab.com/gitlab-org/gitlab, and https://gitlab.com/gitlab-org/gitlab-ce will become https://gitlab.com/gitlab-org/gitlab-foss.

Why rename gitlab-ce to gitlab-foss?

Using "gitlab" and "gitlab-ce" would be confusing, so we decided to rename gitlab-ce to gitlab-foss to make the purpose of this FOSS repository more clear

I created a merge requests for CE, and this got closed. What do I need to do?

You will need to create a merge request at https://gitlab.com/gitlab-org/gitlab-ee/. This link will eventually redirect to https://gitlab.com/gitlab-org/gitlab.

How does the licensing work in this new setup?

Everything in the ee/ directory is proprietary. Everything else is free and open source software. If your merge request does not change anything in the ee/ directory, the process of contributing changes is the same as when using the gitlab-ce repository.

Will you accept merge requests on the gitlab-ce/gitlab-foss project after it has been renamed?

No. Merge requests submitted to this project will be closed automatically.

Will I still be able to view old issues and merge requests in gitlab-ce/gitlab-foss?

Yes.

How will this affect users of GitLab CE using Omnibus?

No changes will be necessary, as the packages built remain the same.

How will this affect users of GitLab CE that build from source?

Once the project has been renamed, you will need to change your Git remotes to use this new URL. GitLab will take care of redirecting Git operations so there is no hard deadline, but we recommend doing this as soon as the projects have been renamed.

Where can I see a timeline of the remaining steps?

https://gitlab.com/gitlab-org/gitlab-ee/issues/13304

Will community contributions submitted to the new "gitlab" repository be available in the new gitlab-foss repository?

Yes, these changes will be synced to gitlab-foss automatically, several times per day.

locked this issue

GraphQL API proof of concept: merge request widget endpoint

Designs

Child items 0

Activity

After Looking A Bit More

Frequently Asked Questions

Admin message

Admin message

GraphQL API proof of concept: merge request widget endpoint

Activity

After Looking A Bit More

Frequently Asked Questions