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

router-bridge: Disable Deno snapshotting when docs.rs builds docs #185

Merged
merged 3 commits into from
Sep 15, 2022
Merged

router-bridge: Disable Deno snapshotting when docs.rs builds docs #185

merged 3 commits into from
Sep 15, 2022

Conversation

SimonSapin
Copy link
Contributor

@SimonSapin SimonSapin self-assigned this Sep 15, 2022
@SimonSapin SimonSapin changed the title Disable Deno snapshotting when docs.rs builds docs router-bridge: Disable Deno snapshotting when docs.rs builds docs Sep 15, 2022
@SimonSapin
Copy link
Contributor Author

I’ve tested that cargo doc --features docsrs succeeds on my laptop, but we’ll have to wait until router-bridge is next published to crates.io to see what happens on actual docs.rs build servers.

garypen
garypen reviewed Sep 15, 2022
View reviewed changes
Copy link
Contributor

@garypen garypen left a comment

Choose a reason for hiding this comment

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

Looks good.

federation-2/router-bridge/Cargo.toml Outdated Show resolved Hide resolved
federation-2/router-bridge/build.rs Outdated Show resolved Hide resolved
Geal
Geal reviewed Sep 15, 2022
View reviewed changes
federation-2/router-bridge/Cargo.toml
@@ -45,3 +45,11 @@ tracing-test = "0.2.1"
[build-dependencies]
deno_core = "0.142.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

should deno_core be marked as optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No because cargo doc runs the equivalent of cargo check. It’s only the C++ bits of V8 that are disabled apparently, all the Rust bits of Deno and rusty_v8 and still there.

@SimonSapin SimonSapin enabled auto-merge (squash) September 15, 2022 12:29
@SimonSapin SimonSapin merged commit dfe5148 into main Sep 15, 2022
@SimonSapin SimonSapin deleted the simon/docs-rs branch September 15, 2022 12:38
@abernix abernix mentioned this pull request Sep 16, 2022
Merged
o0Ignition0o pushed a commit to apollographql/router that referenced this pull request Sep 16, 2022
@abernix @bnjjj
release: v1.0.0-rc.1 (#1810)
f3f27d7 
> **Note**
> We're almost to 1.0! We've got a couple relatively small breaking
changes to the configuration for this release (none to the API) that
should be relatively easy to adapt to and a number of bug fixes and
usability improvements.

## ❗ BREAKING ❗

### Change `headers` propagation configuration ([PR
#1795](#1795))

While it wasn't necessary today, we want to avoid a necessary breaking
change in the future by proactively making room for up-and-coming work.
We've therefore introduced another level into the `headers`
configuration with a `request` object, to allow for a `response` (see
[Issue #1284](#1284)) to
be an _additive_ feature after 1.0.

A rough look at this should just be a matter of adding in `request` and
indenting everything that was inside it:

```patch
headers:
    all:
+     request:
          - remove:
              named: "test"
```

The good news is that we'll have `response` in the future! For a full
set of examples, please see the [header propagation
documentation](https://www.apollographql.com/docs/router/configuration/header-propagation/).

By @bnjjj in #1795

### Bind the Sandbox on the same endpoint as the Supergraph, again
([Issue #1785](#1785))

We have rolled back an addition that we released in this week's
`v1.0.0-rc.0` which allowed Sandbox (an HTML page that makes requests to
the `supergraph` endpoint) to be on a custom socket. In retrospect, we
believe it was premature to make this change without considering the
broader impact of this change which ultimately touches on CORS and some
developer experiences bits. Practically speaking, we may not want to
introduce this because it complicates the model in a number of ways.

For the foreseeable future, Sandbox will continue to be on the same
listener address as the `supergraph` listener.

It's unlikely anyone has really leaned into this much already, but if
you've already re-configured `sandbox` or `homepage` to be on a custom
`listen`-er and/or `path` in `1.0.0-rc.0`, here is a diff of what you
should remove:

```diff
sandbox:
-  listen: 127.0.0.1:4000
-  path: /
  enabled: false
homepage:
-  listen: 127.0.0.1:4000
-  path: /
  enabled: true
```

Note this means you can either enable the `homepage`, or the `sandbox`,
but not both.

By @o0Ignition0o in #1796

## 🚀 Features

### Automatically check "Return Query Plans from Router" checkbox in
Sandbox ([Issue
#1803](#1803))

When loading Sandbox, we now automatically configure it to toggle the
"Request query plans from Router" checkbox to the enabled position which
requests query plans from the Apollo Router when executing operations.
These query plans are displayed in the Sandbox interface and can be seen
by selecting "Query Plan Preview" from the drop-down above the panel on
the right side of the interface.

By @abernix in #1804

## 🐛 Fixes

### Fix `--dev` mode when no configuration file is specified ([Issue
#1801](#1801)) ([Issue
#1802](#1802))

We've reconciled an issue where the `--dev` mode flag was being ignored
when running the router without a configuration file. (While many use
cases do require a configuration file, the Router actually doesn't
_need_ a confguration in many cases!)

By @bnjjj in #1808

### Respect `supergraph`'s `path` for Kubernetes deployment probes
([Issue #1787](#1787))

If you've configured the `supergraph`'s `path` property using the Helm
chart, the liveness
and readiness probes now utilize these correctly. This fixes a bug where
they continued to use the _default_ path of `/` and resulted in a
startup failure.

By @damienpontifex in #1788

### Get variable default values from the query for query plan condition
nodes ([PR #1640](#1640))

The query plan condition nodes, generated by the `if` argument of the
`@defer` directive, were
not using the default value of the variable passed in as an argument.

This _also_ fixes _default value_ validations for non-`@defer`'d
queries.

By @Geal in #1640

### Correctly hot-reload when changing the `supergraph`'s `listen`
socket ([Issue
#1814](#1814))

If you change the `supergraph`'s `listen` socket while in `--hot-reload`
mode, the Router will now correctly pickup the change and bind to the
new socket.

By @o0Ignition0o in #1815

## 🛠 Maintenance

### Improve error message when querying non existent field [Issue
#1816](#1816)

When querying a non-existent field you will get a better error message:

```patch
{
  "errors": [
    {
-       "message": "invalid type error, expected another type than 'Named type Computer'"
+       "message": "Cannot query field \"xxx\" on type \"Computer\""
    }
  ]
}
```

By @bnjjj in #1817

### Update `apollo-router-scaffold` to use the published `apollo-router`
crate [PR #1782](#1782)

Now that `apollo-router` is released on
[crates.io](https://crates.io/crates/apollo-router), we have updated the
project scaffold to rely on the published crate instead of Git tags.

By @o0Ignition0o in #1782

### Refactor `Configuration` validation [Issue
#1791](#1791)

Instantiating `Configuration`s is now fallible, because it will run
consistency checks on top of the already run structure checks.

By @o0Ignition0o in #1794

### Refactor response-formatting tests
[#1798](#1798)

Rewrite the response-formatting tests to use a builder pattern instead
of macros and move the tests to a separate file.

By @Geal in #1798

## 📚 Documentation

### Add `rustdoc` documentation to various modules ([Issue
#799](#799))

Adds documentation for:

- `apollo-router/src/layers/instrument.rs`
- `apollo-router/src/layers/map_first_graphql_response.rs`
- `apollo-router/src/layers/map_future_with_request_data.rs`
- `apollo-router/src/layers/sync_checkpoint.rs`
- `apollo-router/src/plugin/serde.rs`
- `apollo-router/src/tracer.rs`

By @garypen in #1792

### Fixed `docs.rs` publishing error from our last release

During our last release we discovered for the first time that our
documentation wasn't able to compile on the [docs.rs](https://docs.rs)
website, leaving our documentation in a [failed
state](https://docs.rs/crate/apollo-router/1.0.0-rc.0/builds/629200).

While we've reconciled _that particular problem_, we're now being
affected by
[this](https://docs.rs/crate/router-bridge/0.1.7/builds/629895) internal
compiler errors (ICE) that [is
affecting](rust-lang/rust#101844) anyone using
`1.65.0-nightly` builds circa today. Since docs.rs uses `nightly` for
all builds, this means it'll be a few more days before we're published
there.

With thanks to @SimonSapin in
apollographql/federation-rs#185

Co-authored-by: Coenen Benjamin <[email protected]>
SimonSapin added a commit to apollographql/router that referenced this pull request Sep 20, 2022
@SimonSapin
Disable Deno snapshotting in router-bridge when docs.rs builds docs
628c509 
apollographql/federation-rs#185 was not sufficient
since each package being published to crates.io needs to have its own
`[package.metadata.docs.rs]` config.
SimonSapin added a commit to apollographql/router that referenced this pull request Sep 20, 2022
@SimonSapin
Disable Deno snapshotting in router-bridge when docs.rs builds docs
31c0c16 
apollographql/federation-rs#185 was not sufficient
since each package being published to crates.io needs to have its own
`[package.metadata.docs.rs]` config.
@SimonSapin SimonSapin mentioned this pull request Sep 20, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Geal Geal Geal approved these changes

@garypen garypen garypen left review comments

@o0Ignition0o o0Ignition0o o0Ignition0o approved these changes

@EverlastingBugstopper EverlastingBugstopper Awaiting requested review from EverlastingBugstopper

Assignees

@SimonSapin SimonSapin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SimonSapin @garypen @Geal @o0Ignition0o