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

permissions ui improvments (close #1068) #1205

Merged
merged 2 commits into from
Dec 15, 2018

Conversation

arvi3411301
Copy link
Member

Description

What component does this PR affect?

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Requires changes from other components? If yes, please mark the components:

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Related Issue

#1068

Solution and Design

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs update
  • Community content

Checklist:

  • I have read the contributing guide and my code conforms to the guidelines.
  • This change requires a change in the documentation.
  • I have updated the documentation accordingly.
  • I have added required tests.

@arvi3411301 arvi3411301 added s/do-not-merge Do not merge this pull request to master c/console Related to console labels Dec 13, 2018
@hasura-bot
Copy link
Contributor

Review app for commit 4d4991d deployed to Heroku: https://hge-ci-pull-1205.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1205-4d4991d

@karthikvt26 karthikvt26 added s/ok-to-merge Status: This pull request can be merged to master and removed s/do-not-merge Do not merge this pull request to master labels Dec 13, 2018
@hasura-bot
Copy link
Contributor

Review app for commit b5bde6d deployed to Heroku: https://hge-ci-pull-1205.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1205-b5bde6d

@shahidhk shahidhk changed the title console: permissions UI improvments, close #1068 permissions ui improvments (close #1068) Dec 15, 2018
@shahidhk shahidhk merged commit 58cfbed into hasura:master Dec 15, 2018
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-1205.herokuapp.com is deleted

@arvi3411301 arvi3411301 deleted the issue-1068 branch December 17, 2018 07:48
hasura-bot pushed a commit that referenced this pull request Oct 7, 2024
…var (#1205)

### What
Currently if you want v3-engine to accept metadata that is only a
partial supergraph (ie missing some subgraphs), you must set the
PARTIAL_SUPERGRAPH env var. Builds sent to MBS get sent to a special
endpoints `/validate/partial` and `/build/partial` that runs the engine
build process with that configuration option set.

This results in a terrible user experience for local builds, because MBS
will accept a partial supergraph and yield build artifacts, but
v3-engine will refuse to run them unless you set that env var.

This PR removes that env var and creates a new OpenDD flag called
`allow_partial_supergraph`. When MBS's `/validate/partial` endpoint is
used, that flag is set in the build artifacts. v3-engine then looks at
that flag to enable partial supergraph mode. This means a
`/build/partial` build via MBS just works when you run it locally via
v3-engine.

### How

`metadata_resolve`'s `Configuration.allow_unknown_subgraphs` has been
removed, and `metadata_resolve` now looks at OpenDD's new
`Flags.allow_partial_supergraph` instead.

In MBS, usage of `ValidationMode` (the enum that enables partial builds)
previously used to set `Configuration.allow_unknown_subgraphs`; now it
is used in `compute_open_dds_flags` in order to set
`Flags.allow_partial_supergraph`.

The existing metadata_resolve test that tested partial supergraphs has
been modified to use the flags rather than the removed configuration
option
(`crates/metadata-resolve/tests/passing/missing_subgraph_when_ignoring_unknown_subgraphs`).

A new `metadata_resolve` test has been added that checks that comparable
relationships in `BooleanExpressionType` properly respects the
`allow_partial_subgraphs` flag (this functionality was added in #1182).

V3_GIT_ORIGIN_REV_ID: 2c984eb791263a1fb0606c6c44a2a1ae4a5e7370
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/console Related to console s/ok-to-merge Status: This pull request can be merged to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants