When running using the "new" style (with a HGE binary, not a URL), a new PostgreSQL metadata and source database are created for each test. When we get this into CI, this should drastically reduce the flakiness.
I have also enabled parallelization by default when using `run-new.sh`. It's much faster.
I had to basically rewrite _server/tests-py/test_graphql_read_only_source.py_ so that it does two different things depending on how it's run. It's unfortunate, but it should eventually go away.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6879
GitOrigin-RevId: a121b9035f8da3e61a3e36d8b1fbc6ccae918fad
This enables sharing the Docker Compose-based database configuration across the Haskell-based API tests and the legacy Python integration tests.
Why? Because we depend on different database versions and I keep running out of disk space. I am far too lazy to buy another disk and set up my operating system _again_.
The files in question are:
- _docker-compose/databases.yaml_, which is the base specification for the databases
- _docker-compose.yml_, used by the API tests locally (and for other manual testing), which extends the above
- _.buildkite/docker-compose-files/test-oss-server-hspec.yml_, used by the API tests in CI, which extends _databases.yaml_
- _server/tests-py/docker-compose.yml_, used by the Python integration tests
The changes are summarized as follows:
1. The following snippets are moved from _docker-compose/databases.yaml_ to _docker-compose.yml_ and _.buildkite/docker-compose-files/test-oss-server-hspec.yml_, as they're not strictly necessary for other forms of testing:
- the fixed port mappings (in the range 65000–65010)
- the PostgreSQL initialization
- the SQL Server initialization
2. Environment variables are used a little more in health checks and initialization scripts, as usernames, passwords, etc. can be overridden.
3. The volumes in _docker-compose/databases.yaml_ are made anonymous (unnamed), and the names are only specified in _docker-compose.yml_. We don't need to do this elsewhere.
- For extra fun, I have removed all named volumes from the CI Docker Compose files, as they seem to be unnecessary.
4. _server/tests-py/docker-compose.yml_ now depends on _docker-compose/databases.yaml_.
- This was the point.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6864
GitOrigin-RevId: f22f2839716f543ce8a62f890da244de7e23abaa
`ssl.wrap_socket` is deprecated in favor of `SSLContext.wrap_socket`.
Also throws in a quick speed improvement to _server/tests-py/run.sh_ on x86_64.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6498
GitOrigin-RevId: 7bbe5f86daf45677e2a39cfcfe183794ffcd2954
If the tests are run with specific ports assigned to specific services,
set through the environment variables, we continue to use those ports.
We just don't hard-code them now, we pick them up from the environment
variables.
However, if the environment variables are not set, we generate a random
port for each service. This allows us to run multiple tests in parallel
in the future, independently.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6218
GitOrigin-RevId: 3d2a1880bf67544c848951888ce7b4fa1ba379dc
This installs the ODBC Driver 18 for SQL Server in all our shipped Docker images, and update our tests and documentation accordingly.
This version supports arm64, and therefore can run natively (or via Docker) on macOS on aarch64.
`msodbcsql17` is still installed in production-targeted Docker images so that users do not _have_ to migrate to the new driver.
Nix expressions are packaged for the new driver, as it is not yet available in nixpkgs.
In this version, [the default encryption setting was changed from "no" to "yes"](https://techcommunity.microsoft.com/t5/sql-server-blog/odbc-driver-18-0-for-sql-server-released/ba-p/3169228). In addition, "mandatory" and "optional" were added as synonyms for "yes" and "no" respectively.
I have therefore modified all connection strings in tests to specify `Encrypt=optional` (and changed some from `Encrypt=no`). I chose "optional" rather than "no" because I feel it's more honest; these connection strings will work with or without an encrypted connection.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6241
GitOrigin-RevId: 959f88dd1f271ef06a3616bc46b358f364f6cdfd
We use a helper service to start a webhook-based authentication service for some tests. This moves the initialization of the service out of _test-server.sh_ and into the Python test harness, as a fixture.
In order to do this, I had to make a few changes. The main deviation is that we no longer run _all_ tests against an HGE with this authentication service, just a few (those in _test_webhook.py_). Because this reduced coverage, I have added some more tests there, which actually cover some areas not exacerbated elsewhere (mainly trying to use webhook credentials to talk to an admin-only endpoint).
The webhook service can run both with and without TLS, and decide whether it's necessary to skip one of these based on the arguments passed and how HGE is started, according to the following logic:
* If a TLS CA certificate is passed in, it will run with TLS, otherwise it will skip it.
* If HGE was started externally and a TLS certificate is provided, it will skip running without TLS, as it will assume that HGE was configured to talk to a webhook over HTTPS.
* Some tests should only be run with TLS; this is marked with a `tls_webhook_server` marker.
* Some tests should only be run _without_ TLS; this is marked with a `no_tls_webhook_server` marker.
The actual parameterization of the webhook service configuration is done through test subclasses, because normal pytest parameterization doesn't work with the `hge_fixture_env` hack that we use. Because `hge_fixture_env` is not a sanctioned way of conveying data between fixtures (and, unfortunately, there isn't a sanctioned way of doing this when the fixtures in question may not know about each other directly), parameterizing the `webhook_server` fixture doesn't actually parameterize `hge_server` properly. Subclassing forces this to work correctly.
The certificate generation is moved to a Python fixture, so that we don't have to revoke the CA certificate for _test_webhook_insecure.py_; we can just generate a bogus certificate instead. The CA certificate is still generated in the _test-server.sh_ script, as it needs to be installed into the OS certificate store.
Interestingly, the CA certificate installation wasn't actually working, because the certificates were written to the wrong location. This didn't cause any failures, as we weren't actually testing this behavior. This is now fixed with the other changes.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6363
GitOrigin-RevId: 0f277d374daa64f657257ed2a4c2057c74b911db
This helps with running the tests locally on a recent Mac.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6304
Co-authored-by: Daniel Harvey <4729125+danieljharvey@users.noreply.github.com>
GitOrigin-RevId: eb5c6a185f68b216c94df2581acf71906cce7872
This makes it possible for the test harness to start the test JWK server and the test remote schema server.
In order to do this, we still generate the TLS certificates in the test script (because we need to install the generated CA certificate in the OS certificate store), and then pass the certificate and key paths into the test runner.
Because we are still using _test-server.sh_ for now, we don't use the JWK server fixture in that case, as HGE needs the JWK server to be up and running when it starts. Instead, we keep running it outside (for now).
This is also the case for the GraphQL server fixture when we are running the server upgrade/downgrade tests.
I have also refactored _graphql_server.py_ so there isn't a global `HGE_URLS` value, but instead the value is passed through.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6303
GitOrigin-RevId: 06f05ff674372dc5d632e55d68e661f5c7a17c10
This updates _docker-compose.yml_ to use the new image tags, and updates _run.sh_ accordingly.
While I was at it, I also added a `docker compose pull` instruction to make sure that we don't have surprises half-way through the script, and a few `echo` lines for clarity.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6235
GitOrigin-RevId: 3855f6898bd3e906c5f423d9d0d6a7031de3777a
This PR is the result of running the following commands:
```bash
$ git grep -l '".* : "' -- '*.hs' | xargs sed -i -E 's/(".*) : "/\1: "/'
$ scripts/dev.sh test --integration --accept
```
Also manually fixed a few tests and docs
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6148
GitOrigin-RevId: cf8b87605d41d9ce86613a41ac5fd18691f5a641
When we run the HGE server inside the test harness, it needs to run with
an admin secret for some tests to make sense. This tags each test that
requires an admin secret with `pytest.mark.admin_secret`, which then
generates a UUID and injects that into both the server and the test case
(if required).
It also simplifies the way the test harness picks up an existing admin
secret, allowing it to use the environment variable instead of requiring
it via a parameter.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6120
GitOrigin-RevId: 55c5b9e8c99bdad9c8304098444ddb9516749a2c
This teaches `hge_server` how to run more tests, thanks to `hge_env`.
It also simplifies the logic a bit more.
I have also modified _run.sh_ and _docker-compose.yml_ so we can run multiple test suites, one after another.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6105
GitOrigin-RevId: eff009362eb6bb90c07cedaf96dfe6ec9336ff32
This has two purposes:
* When running the Python integration tests against a running HGE instance, with `--hge-url`, it will check the environment variables available and actively skip the test if they aren't set. This replaces the previous ad-hoc skip behavior.
* More interestingly, when running against a binary with `--hge-bin`, the environment variables are passed through, which means different tests can run with different environment variables.
On top of this, the various services we use for testing now also provide their own environment variables, rather than expecting a test script to do it.
In order to make this work, I also had to invert the dependency between various services and `hge_ctx`. I extracted a `pg_version` fixture to provide the PostgreSQL version, and now pass the `hge_url` and `hge_key` explicitly to `ActionsWebhookServer`.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6028
GitOrigin-RevId: 16d866741dba5887da1adf4e1ade8182ccc9d344
NPM v7 uses a new (backwards-compatible) lockfile format. This upgrades all our various _package-lock.json_ files to use the new format.
It's much more verbose so that NPM can be a lot faster.
I figured it was cleaner to do this once in a separate PR rather than upgrading them in combination with adding or upgrading a new dependency.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5869
GitOrigin-RevId: 322fb63b96e2d873a4a3cc05fa6c7afa414716ce
This adds support for running the Python integration tests for MSSQL and Citus just as in CI, as follows:
```
./server/tests-py/run.sh backend-mssql
./server/tests-py/run.sh backend-citus
```
These run the named CI jobs, providing the appropriate backend.
(In reality, all backends are always provided, which is much simpler.)
It also provides the various databases to _server/tests-py/run-new.sh_, though the tests fail as they don't properly initialize the sources. (This will be fixed in the future by provisioning sources in the test framework itself.)
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5997
GitOrigin-RevId: c276a4779a35bb538ef0dc02ac8b7cb2d5a8dec5
This fixes a few issues so that we can run `./server/tests-py/run.sh backend-bigquery` to run the Python integration tests for BigQuery locally.
* We forward the relevant environment variables to the Docker container.
* We increase the HTTP timeout, as I'm seeing requests taking up to 90s locally.
* We rewrite the setup so that it avoids `INSERT INTO`, which is not available using the BigQuery free tier. Instead, we use `CREATE TABLE ... AS SELECT ...`. This is the same method used by the Haskell integration tests.
We also capture local server output in a volume so it's easier to figure out what went wrong later.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5921
GitOrigin-RevId: c628f8c08a84f2582958659ab6d6494832471f6f
This makes two changes to the Docker Compose files that we use for local testing:
1. We disable `fsync`. On my machine, this decreases the time taken to create a new database from ~5s to less than 0.1s. The trade-off is that you might lose data, which we don't care about, as this is for testing.
2. We increase the maximum number of connections from the default, 100, to 1000. This allows us to run more tests in parallel without hitting connection limits.
These changes won't have any meaningful effect for now; they simply allow us to parallelize tests against PostgreSQL in the future.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5892
GitOrigin-RevId: 5d0d0ab37fdfbf4c9e20084d3cbedf647f54a04e
This argument allows the user to specify how to run HGE, rather than starting it beforehand. The runner will start a new instance of HGE for each test class.
This does not provide isolation, as the database is still re-used, but it helps us get closer.
You can try it yourself by executing:
```
$ cabal build graphql-engine:exe:graphql-engine
$ ./server/tests-py/run-new.sh
```
This doesn't affect CI at all.
I also fixed a few warnings flagged by Pylance.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5881
GitOrigin-RevId: ea6f0fd631a2c278b2c6b50e9dbdd9d804ebc9d4
Starting it and stopping it for the various tests that actually use it.
There are only a few.
This also removes some dead code and fixes warnings in _test_webhook_request_context.py_.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5846
GitOrigin-RevId: 7760467f9de7b1f9718e7482275c298eeaa3ad3a
This allows a developer, through Docker, to run the Python integration tests in pretty much exactly the same way as CI does.
Allowing us to more readily diagnose issues locally.
I'm hoping this is temporary and we won't need it for too long, but I have found it invaluable over the last few days so I would like to share it.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5818
GitOrigin-RevId: 18876fbbcbe7c5492afdf54d96af45ab2c519b77
Where possible, we start the services on random ports, to avoid
port conflicts when parallelizing tests in the future.
When this isn't possible, we explicitly state the port, and wait for the
service to start. This is typically because the GraphQL Engine has already
started with knowledge of the relevant service passed in through an
environment variable.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5542
GitOrigin-RevId: b51a095b8710e3ff20d1edb13aa576c5272a5565
We plan on creating the source database dynamically, in the test setup.
This means that (a) we cannot assume that the metadata database and the
source database are the same, and (b) we need to drop and re-add the
source in code, not in YAML.
This changeset prepares the code for the introduction of a separate
source database, but doesn't go there yet. The separation is already
done but is too big to review in one go, so I have split this out.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5508
GitOrigin-RevId: b497a83ac4a100371762c2515c87ee3760d8d4ab
This splits two naming convention tests into four classes (and four YAML
files), which might seem overkill, but allows us to provision sources
declaratively in the future. As each class will require a custom source
configuration, we are able to annotate them accordingly, which means the
test cases are decoupled from the source database URL, letting us
generate a new database for each test case and automatically add it as a
source to HGE.
The future changes are already prepared, but this has been extracted out
as it splits the YAML files, which is a large change best reviewed in
isolation.
The test case `test_type_and_field_names` has been split into:
* `TestNamingConventionsTypeAndFieldNamesGraphqlDefault`
* `TestNamingConventionsTypeAndFieldNamesHasuraDefault`
The test case `test_type_and_field_names_with_prefix_and_suffix` has
been split into:
* `TestNamingConventionsTypeAndFieldNamesGraphqlDefaultWithPrefixAndSuffix`
* `TestNamingConventionsTypeAndFieldNamesHasuraDefaultWithPrefixAndSuffix`
The YAML files have been split in the same way. This was fairly trivial
as each test case would add a source, run some tests with
the `graphql_default` naming convention, drop the source, and then
repeat for the `hasura_default` naming convention. I simply split the
file in two. There is a little bit of duplication for provisioning the
various database tables, which I think is worth it.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5496
GitOrigin-RevId: 94825e755c427a5414230f69985b534991b3aad6
This means that if `remote_schemas/nodejs/package.json` changes, the
dependencies will be automatically reinstalled.
It also moves `package-lock.json` to the correct location (in the
directory in which we run `npm install`), and updates it.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5481
GitOrigin-RevId: f3fb431afd19de150f39ec2e4cb6572b896c870f
Making it easier to inject different ones later.
I also included a change to _.prettierignore_ so Visual Studio Code doesn't keep trying to reformat the JavaScript or YAML files in `server/tests-py`, as it can cause diffs to balloon for no obvious benefit.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5456
GitOrigin-RevId: bc6d548708160a328e1e61a00e19be8e124da025
Let's put it in one place.
This is a precursor to moving database provisioning into the Python
integration tests.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5453
GitOrigin-RevId: 5920b0b1177d94496485fcb4e178b946534ee4eb
I'm trying to shore up the Python integration tests to make them more reliable. In doing so, I noticed this.
---
Rather than hard-coding hostnames and ports, we can (and already do) inject these into the HGE process using environment variables.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5255
GitOrigin-RevId: 6bb593999ece42cedef6619f31f9d9b2e39f30ef
When documenting how adding a backend works, the information was a bit
out of date. Updated to link to files from the latest commit to `main`,
at the time of writing.
Also runs the README through the `prettier` autoformatter.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5301
GitOrigin-RevId: df54f95d85156e9f95a4a7788eed93c6359cc81c
For some reason these functions exist in `Backends.Postgres.SQL.Value`.
We don't want to depend on that module here.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5292
GitOrigin-RevId: a09bd3cdb0caf08938bce0728a8d281344c1d4ce
I'm trying to shore up the Python integration tests to make them more reliable. In doing so, I noticed this.
---
It feels a lot more sensible as we never run on more than one backend at a time.
This also removes the `check_file_exists` parameter from the setup functions; it never worked. It was always set to the result of a comparison between a backend name and a function, which was always `False`. Enabling it breaks things.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5254
GitOrigin-RevId: 8718ab21527c2ba0a7205d1c01ebaac1a10be844
Rather than a homebrewed approach, we can use `make` to figure out when
it's necessary to regenerate our venv.
This Makefile will regenerate _requirements.txt_ from
_requirements-top-level.txt_ when the latter is changed.
It will also regenerate the venv when _requirements.txt_ is changed
(i.e. changes are pulled, or it's regenerated as described above).
`make` uses file/directory timestamps to figure out what to rebuild.
This is probably more reliable than expecting people to update a version
number whenever they change a file.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5152
GitOrigin-RevId: 24b27d49bf6c4ba1d57ac38ea38ae278216c6d66
I tried re-freezing _server/tests-py/requirements-top-level.txt_
recently, and discovered that it caused the tests to fail.
This pins a couple of dependencies so that we can safely re-freeze.
Specifically:
- `cryptography` is pinned at v3.*
- `graphene` is pinned at v2.*
- `PyJWT` is pinned at v2.3.*
- `websocket-client` is pinned at v0.56.0 (this was done in
_requirements.txt_ already, but that file is supposed to be
regenerated)
Upgrading `SQLAlchemy` required changing PostgreSQL URLs to use
"postgresql://" as the URL scheme, not "postgres://".
Updating `ruamel.yaml` caused a few tests to fail as we are passing
`ruamel.yaml.scalarstring.LiteralScalarString` values as header values.
This is fixed by explicitly converting header values to strings.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5120
GitOrigin-RevId: 9c12a3013c3d1f23dddbe781037663838b23f6f5
This introduces an `ErrorMessage` newtype which wraps `Text` in a manner which is designed to be easy to construct, and difficult to deconstruct.
It provides functionality similar to `Data.Text.Extended`, but designed _only_ for error messages. Error messages are constructed through `fromString`, concatenation, or the `toErrorValue` function, which is designed to be overridden for all meaningful domain types that might show up in an error message. Notably, there are not and should never be instances of `ToErrorValue` for `String`, `Text`, `Int`, etc. This is so that we correctly represent the value in a way that is specific to its type. For example, all `Name` values (from the _graphql-parser-hs_ library) are single-quoted now; no exceptions.
I have mostly had to add `instance ToErrorValue` for various backend types (and also add newtypes where necessary). Some of these are not strictly necessary for this changeset, as I had bigger aspirations when I started. These aspirations have been tempered by trying and failing twice.
As such, in this changeset, I have started by introducing this type to the `parseError` and `parseErrorWith` functions. In the future, I would like to extend this to the `QErr` record and the various `throwError` functions, but this is a much larger task and should probably be done in stages.
For now, `toErrorMessage` and `fromErrorMessage` are provided for conversion to and from `Text`, but the intent is to stop exporting these once all error messages are converted to the new type.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5018
GitOrigin-RevId: 84b37e238992e4312255a87ca44f41af65e2d89a
We have a lot of `assert st_code == 200` scattered about. This is a
problem because (a) it makes the code harder to parse and (b) the error
message is lacking; I have seen a few flaky tests which were impossible
to diagnose because I didn't know what the response _should_ be.
This reduces the number of places in which we perform this assertion
(moving most of them to `HGECtx.execute_query`), so that we can have a
better chance of seeing a useful error message on test failure.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4957
GitOrigin-RevId: 3ff388bccf49f96569aa6b7db85266a0c5ee27ea
This makes it easier to run the tests when `cd`-ing into the directory,
and also takes care of cleaning up the environment when you exit the
directory.
It also helps editors with direnv support find the relevant libraries
so that they can perform more helpful analysis.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4963
GitOrigin-RevId: d253c054c09c1bf7017ade9f7af414b56ee26fe0
This improves `parseJSONPath` and `encodeJSONPath` to encode special characters appropriately by delegating to Aeson.
This also makes a couple of improvements to `encodeJSONPath`.
1. The function is moved from `Hasura.Base.Error` to `Data.Parser.JSONPath`. This still doesn't seem too appropriate but it is somewhat better. I am basing this on the fact that its test cases already lived in `Data.Parser.JSONPathSpec`.
2. It now returns `Text`, not `String`.
4. It quotes strings with double quotes (`"`) rather than single quotes (`'`), just like JSON.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4935
GitOrigin-RevId: bf44353cd740500245f2e38907a7d6263ae0291c
I found a couple of flaws in `TestNamingConventions` and friends:
1. We had two test cases with the same name, which means one of them
would be overwritten. Renamed to avoid conflict.
2. The `skipif` check for
`TestNamingConventionWithoutExperimentalFeature` seemed broken. I
have fixed it by making it line up with its `reason`, and extracted
some logic out into a function to avoid duplication.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4958
GitOrigin-RevId: f572d86c893135839dbaba70bf89984bc9d79331
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
…fix #5426"
This reverts commit f85742318167d1e51f463c45fcd00f26269c2555.
## Description ✍️
With this commit there is the possiblity that you could get conflicting
type definitions with remote schemas. Reverting for now as we determine
a solution. At which point we will add this back in.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4879
Co-authored-by: Gil Mizrahi <8547573+soupi@users.noreply.github.com>
GitOrigin-RevId: 932b4a9226717c826d4bde7e375695354cee8c0c
The code that builds the GraphQL schema, and `buildGQLContext` in particular, is partial: not every value of `(ServerConfigCtx, GraphQLQueryType, SourceCache, HashMap RemoteSchemaName (RemoteSchemaCtx, MetadataObject), ActionCache, AnnotatedCustomTypes)` results in a valid GraphQL schema. When it fails, we want to be able to return better error messages than we currently do.
The key thing that is missing is a way to trace back GraphQL type information to their origin from the Hasura metadata. Currently, we have a number of correctness checks of our GraphQL schema. But these correctness checks only have access to pure GraphQL type information, and hence can only report errors in terms of that. Possibly the worst is the "conflicting definitions" error, which, in practice, can only be debugged by Hasura engineers. This is terrible DX for customers.
This PR allows us to print better error messages, by adding a field to the `Definition` type that traces the GraphQL type to its origin in the metadata. So the idea is simple: just add `MetadataObjId`, or `Maybe` that, or some other sum type of that, to `Definition`.
However, we want to avoid having to import a `Hasura.RQL` module from `Hasura.GraphQL.Parser`. So we instead define this additional field of `Definition` through a new type parameter, which is threaded through in `Hasura.GraphQL.Parser`. We then define type synonyms in `Hasura.GraphQL.Schema.Parser` that fill in this type parameter, so that it is not visible for the majority of the codebase.
The idea of associating metadata information to `Definition`s really comes to fruition when combined with hasura/graphql-engine-mono#4517. Their combination would allow us to use the API of fatal errors (just like the current `MonadError QErr`) to report _inconsistencies_ in the metadata. Such inconsistencies are then _automatically_ ignored. So no ad-hoc decisions need to be made on how to cut out inconsistent metadata from the GraphQL schema. This will allow us to report much better errors, as well as improve the likelihood of a successful HGE startup.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4770
Co-authored-by: Samir Talwar <47582+SamirTalwar@users.noreply.github.com>
GitOrigin-RevId: 728402b0cae83ae8e83463a826ceeb609001acae
When pytest rewrites assertions to make them more useful, it also
truncates long assertion messages in the middle, often obscuring the
actual error.
Disabling this allows us to see the full message, which should hopefully
allow us to see the actual error.
In order to support this, we need to make sure that custom assertion
messages include the actual assertion information, as this will no
longer be rendered by pytest.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4835
GitOrigin-RevId: de6839a3b40d0edc7cb96e46625eebca0aaf0c95
(Work here originally done by awjchen, rebased and fixed up for merge by
jberryman)
This is part of a merge train towards GHC 9.2 compatibility. The main
issue is the use of the new abstract `KeyMap` in 2.0. See:
https://hackage.haskell.org/package/aeson-2.0.3.0/changelog
Alex's original work is here:
#4305
BEHAVIOR CHANGE NOTE: This change causes a different arbitrary ordering
of serialized Json, for example during metadata export. CLI users care
about this in particular, and so we need to call it out as a _behavior
change_ as we did in v2.5.0. The good news though is that after this
change ordering should be more stable (alphabetical key order).
See: https://hasurahq.slack.com/archives/C01M20G1YRW/p1654012632634389
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4611
Co-authored-by: awjchen <13142944+awjchen@users.noreply.github.com>
GitOrigin-RevId: 700265162c782739b2bb88300ee3cda3819b2e87
### Description
This PR is a first step in a series of cleanups of action relationships. This first step does not contain any behavioral change, and it simply reorganizes / prunes / rearranges / documents the code. Mainly:
- it divides some files in RQL.Types between metadata types, schema cache types, execution types;
- it renames some types for consistency;
- it minimizes exports and prunes unnecessary types;
- it moves some types in places where they make more sense;
- it replaces uses of `DMap BackendTag` with `BackendMap`.
Most of the "movement" within files re-organizes declarations in a "top-down" fashion, by moving all TH splices to the end of the file, which avoids order or declarations mattering.
### Optional list types
One main type change this PR makes is a replacement of variant list types in `CustomTypes.hs`; we had `Maybe [a]`, or sometimes `Maybe (NonEmpty a)`. This PR harmonizes all of them to `[a]`, as most of the code would use them as such, by doing `fromMaybe []` or `maybe [] toList`.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4613
GitOrigin-RevId: bc624e10df587eba862ff27a5e8021b32d0d78a2
## Motivation
This PR rewrites most of Relay to achieve the following:
- ~~fix a bug in which the same node id could refer to two different tables in the schema~~
- remove one of the few remaining uses of the source cache in the schema building code
In doing so, it also:
- simplifies the `BackendSchema` class by removing `node` from it,
- makes it much easier for other backends to support Relay,
- documents, re-organizes, and clarifies the code.
## Description
This PR introduces a new `NodeId` version ~~, and adapts the Postgres code to always generate this V2 version~~. This new id contains the source name, in addition to the table name, in order to disambiguate similar table names across different sources (which is now possible with source customization). In doing so, it now explicitly handles that case for V1 node ids, and returns an explicit error message instead of running the risk of _silently returning the wrong information_.
Furthermore, it adapts `nodeField` to support multiple backends; most of the code was trivial to generalize, and as a result it lowers the cost of entry for other backends, that now only need to support `AFNodeId` in their translation layer.
Finally, it removes one more cycle in the schema building code, by using the same trick we used for remote relationships instead of using the memoization trick of #4576.
## Remaining work
- ~~[ ]write a Changelog entry~~
- ~~[x] adapt all tests that were asserting on an old node id~~
## Future work
This PR was adapted from its original form to avoid a breaking change: while it introduces a Node ID V2, we keep generating V1 IDs and the parser rejects V2 IDs. It will be easy to make the switch at a later data in a subsequent PR.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4593
GitOrigin-RevId: 88e5cb91e8b0646900547fa8c7c0e1463de267a1
