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

fix: 👽 Added support for nodes query #217

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

karatekaneen
Copy link
Contributor

Ent (sometimes?) generates a "nodes" query to fetch multiple nodes in batch in the latest version . This was not compatible with Bramble so I fixed that.

The queries that I get generated in my project looks like this:

type Query {
  """Fetches an object given its ID."""
  node(
    """ID of the object."""
    id: ID!
  ): Node
  """Lookup nodes by a list of IDs."""
  nodes(
    """The list of node IDs."""
    ids: [ID!]!
  ): [Node]!
}

Maybe related to #96, not sure

Ent (sometimes?) generates a "nodes" query to fetch multiple nodes in batch in the latest version . This was not compatible with Bramble so I fixed that

Maybe related to movio#96, not sure
@karatekaneen karatekaneen requested a review from a team as a code owner July 23, 2023 17:01
@pkqk
Copy link
Member

pkqk commented Jul 24, 2023

Hi @karatekaneen is this the ent you mean? Can you post an example of generating the graphql nodes field.

The support for the Relay Node interface is relatively unused at the moment, it's left over from an initial prototype design of how our federation model would work.

@codecov-commenter
Copy link

codecov-commenter commented Jul 24, 2023

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (07dc77d) 72.36% compared to head (db081a9) 72.43%.
Report is 4 commits behind head on main.

Files Patch % Lines
validate.go 77.19% 11 Missing and 2 partials ⚠️
config.go 0.00% 5 Missing ⚠️
executable_schema.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #217      +/-   ##
==========================================
+ Coverage   72.36%   72.43%   +0.07%     
==========================================
  Files          27       27              
  Lines        2790     2848      +58     
==========================================
+ Hits         2019     2063      +44     
- Misses        630      644      +14     
  Partials      141      141              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@karatekaneen
Copy link
Contributor Author

@pkqk Yes, it's Ent with entgql I'm referring to. The problem was that when I was trying to add my service to Bramble I got an error with something like "unknown type Node" which I wanted to get rid off.

My Ent schema is currently something of a kitchensink but it looks like this:

package schema

import (
	"entgo.io/contrib/entgql"
	"entgo.io/contrib/entproto"
	"entgo.io/ent"
	"entgo.io/ent/dialect/entsql"
	"entgo.io/ent/schema"
	"entgo.io/ent/schema/field"
)

type FormData struct {
	ent.Schema
}

func (FormData) Annotations() []schema.Annotation {
	return []schema.Annotation{
		entsql.Annotation{Table: "data"},
		entgql.RelayConnection(),
		entgql.Directives(entgql.Directive{Name: "boundary"}),
		entgql.QueryField(),
		entproto.Message(),
		entproto.Service(entproto.Methods(entproto.MethodGet)),
	}
}

// Fields of the FormData.
func (FormData) Fields() []ent.Field {
	return []ent.Field{
		field.Int("object_id").
			StorageKey("obj_id").
			Annotations(entproto.Field(2)),

		field.String("module").
			StorageKey("data_mod").
			Annotations(entproto.Field(3)),

		field.Int("type").
			StorageKey("data_type").
			Annotations(entproto.Field(4)),
	}
}

// Edges of the FormData.
func (FormData) Edges() []ent.Edge {
	return []ent.Edge{}
}

And the relevant parts of GraphQL that gets generated looks like this:

type FormData implements Node @boundary {
  id: ID!
  objectID: Int!
  module: String!
  type: Int!
}

interface Node @goModel(model: "bitbucket.org/company/entdemo/ent.Noder") {
  id: ID!
}

type Query {
  node(id: ID!): Node
  nodes(ids: [ID!]!): [Node]!
  formDataSlice(
    after: Cursor
    first: Int
    before: Cursor
    last: Int
    where: FormDataWhereInput
  ): FormDataConnection!
}

So, just to be clear. I don't mind the Relay Node interface to be unused. It's just that I wanted to be able to hook up my projects to Bramble without having to filter out the incompatible query from every schema in the service query.

@karatekaneen
Copy link
Contributor Author

@pkqk Had any chance to take a look at this?

@pkqk
Copy link
Member

pkqk commented Oct 17, 2023

Hi @karatekaneen I haven't had a chance to review it yet, though your use case makes more sense now. I might be able to find time end of next week. Are you ok running your fork in the mean time?

@karatekaneen
Copy link
Contributor Author

@pkqk It's ok to run the fork for now. Just wanted to avoid syncing it when you are doing updates.

karatekaneen and others added 2 commits November 28, 2023 23:52
We are using mTLS with custom certificates and need to override the default http transport. It looked like it was working by overwriting the `QueryHTTPClient` field on the config but noticed that it wasnt being respected when new services was instantiated.
@karatekaneen
Copy link
Contributor Author

@pkqk Found a small bug where the custom http.Client wasn't being respected. Fixed that on this branch as well

@pkqk
Copy link
Member

pkqk commented Dec 5, 2023

Thanks @karatekaneen, would you be able to pull it out into it's own PR? I can merge and test that more quickly. We haven't had a chance to test with Ent yet.

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

4 participants