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.

Sign up for GitHub

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

Jump to bottom

feat(composition): add hint levels to output types #102

Open
EverlastingBugstopper opened this issue Mar 31, 2022 · 3 comments · May be fixed by #200
Open

feat(composition): add hint levels to output types #102

EverlastingBugstopper opened this issue Mar 31, 2022 · 3 comments · May be fixed by #200

Comments

@EverlastingBugstopper
Copy link
Contributor

EverlastingBugstopper commented Mar 31, 2022 •
edited
Loading

hints don't need to be addressed! they're helpful to have, but having them as warnings may push folks to want to fix/remove them. we should just call them hints instead and let people keep them around.
these warning prefixes can be found here
@pcmanus
Copy link

pcmanus commented Apr 8, 2022

Btw, apollographql/federation#1683 has been committed to add a level to hints: it's either DEBUG, INFO or WARN. For colors, I'd suggest:

  • yellow for WARN; that's because I'd use red for errors (which we can display starting with a ERROR; the implementation do have separate objects for errors and hints, but ultimately we want to erase that difference with errors just being the ERROR level, and we can start by having it look this way in UX).
  • blue for INFO (?)
  • grey for DEBUG.

Ideally, we'll also want to not print all of those by default. I'd even probably suggest that only errors gets shown, but that for other "hints" we simply show a single summary along the line of:

The composition has <x> WARN, <y> INFO and <z> DEBUG messages. The details have been written to file `composition-message-200220408-2.md`. You can alternative --level=ALL to have them displayed here.

But:

  1. I obviously mean something cleaner, just trying to convey the idea and we can debate if we want to write a file or not, etc...
  2. we should not do all of that in this ticket :D. It'll definitively be great to get the proper levels when printing the hints quickly so users don't interpret all hints are warning, which they often aren't. The rest should certainly be a followup.

Merged
@EverlastingBugstopper EverlastingBugstopper changed the title fed2: print hints as {yellow} HINT: instead of {red} WARN: feat(composition): add hint levels to output types Apr 13, 2022
@EverlastingBugstopper EverlastingBugstopper transferred this issue from apollographql/rover Apr 13, 2022
@EverlastingBugstopper
Copy link
Contributor Author

in the federation-2 js we need to start returning the hint levels as part of our output

dingxiangfei2009 pushed a commit to dingxiangfei2009/federation-rs that referenced this issue Jun 23, 2022
@cecton @abernix
Introduce CircleCI Matrix build for different platforms (apollographq…
d91b6a6 
…l#102)

This cargo cults a lot of stuff from the `federation` repository, which has
had this configuration in place for a long time now.

Granted this does add a lot of lines, most of it is to account for our use
of the query-planner written in Node.js within the Router.  The additional
jockery is to account for various learnings and differences between the
operating systems, including how they have to have Node.js installed.

Aside from also running on macOS and Windows, it has various other benefits
that relate to our use cases:

- Hoists the responsibility of toolchain stuff to CircleCI Orbs' Rust pkg
- Pins the version of Node.js (and npm that we depend on for our query
  planner integration, rather than having them be whatever is within the
  CircleCI node variant of the Rust image.

It's certainly possible that we'll want this in the code itself to
facilitate developing on those environments, but I think it's too soon to
invest heavily in having Node.js as a dependency for long-term.

Closes apollographql#91
Closes apollographql#100

Co-authored-by: Jesse Rosenberger <[email protected]>
pcmanus added a commit to pcmanus/federation-rs that referenced this issue Oct 10, 2022
@pcmanus
Expose hint levels
ea35322 
Composition hints in federation 2 are generated with a "level" (which
can currently be one of of "DEBUG", "INFO" or "WARN"). This commit
modify harmonizer to include those levels in the output and the type
definitions to decode them, and include them in the `supergraph` output
for future use by `rover`.

Fixes apollographql#102
@pcmanus pcmanus linked a pull request Oct 10, 2022 that will close this issue
Open
@pcmanus
Copy link

pcmanus commented Oct 10, 2022

Took a stab at this in #200. I'm far from seasoned in rust so please be as harsh as you can on the code, I welcome the education.

Assuming this get in at some point, that probably need to be followed up by a rover PR to integrate this in rover supergraph compose, and I'm happy to give that a shot at that, but we should probably decide the UX we want first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Expose hint levels pcmanus/federation-rs
2 participants
@pcmanus @EverlastingBugstopper