Commit Graph

27 Commits

Author SHA1 Message Date
rikinsk
8f022a1008 Merge branch 'main' into stable
GitOrigin-RevId: 9eccd863f3285eba2352b685462f9a672c55274c
2023-04-27 06:53:17 +00:00
Gil Mizrahi
4696b60ec7 chore(debugging): convert trace ghc warnings to hlint warnings
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/8700
GitOrigin-RevId: 29b1ee9172bb10823420de9555c3e22f9846facd
2023-04-11 09:06:56 +00:00
Auke Booij
cfd33e0dba ci(server): fix hlint ignore pragmas
Some fixes as a followup to #8311. In particular, some pragmas were referring to a now-nonexistent hlint rule.

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/8569
GitOrigin-RevId: 2a180427b3e93a71c47569ff10b6c8b6b407f7a2
2023-03-30 14:31:59 +00:00
Gil Mizrahi
4b33b17f93 chore(ci): show bigquery error details when requested
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/8525
GitOrigin-RevId: 882f3ee0dbe7776b15d53a66851b2a57b41fad73
2023-03-29 09:03:47 +00:00
Antoine Leblanc
6118b440b4 chore(server): revamp hlint config and add new undefined rule
### Description

This PR adds a new rule to our hlint config, that flags all uses of `undefined`. Any use of `undefined` will therefore be flagged in future PRs. Additionally, this PR cleans up our hlint config, including fixing the `ignore: { group: _ }` block: `group` isn't a valid key for `ignore`, and therefore that `ignore` block was _suppressing all hints_ in the matching files.

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/8311
GitOrigin-RevId: 51b702cfd056588a8dae9db4c9e4471ea039210e
2023-03-20 08:41:59 +00:00
Philip Lykke Carlsen
8d02c88c1a refactor(github, hlint): Add hlint white-list of modules that can import backend instances
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/8133
Co-authored-by: Tom Harding <6302310+i-am-tom@users.noreply.github.com>
GitOrigin-RevId: 2c8aded4e77003d176931432e33e6d4c8cccfc6c
2023-03-02 14:45:57 +00:00
Philip Lykke Carlsen
1af8d53c6f refactor(hlint): Demote Environment-from-OS to a suggestion
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/8092
GitOrigin-RevId: 042aec936638fcb7890ecaa8cb368ed442100904
2023-02-23 21:44:44 +00:00
Philip Lykke Carlsen
e979a39f6d chore: Fix all outstanding hlint hints
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/8042
GitOrigin-RevId: 87c718fa7b09f375ea0e7c2465788ac49f575290
2023-02-20 17:43:28 +00:00
Philip Lykke Carlsen
384bb1b9d4 refactor(server): Add HLint rules discouraging getEnvironment
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/7959
GitOrigin-RevId: c3189553a7f419a46510463dad7985f2953ef3c3
2023-02-20 13:45:24 +00:00
Philip Lykke Carlsen
9bc1ff1d8e Structured logging in test-harness
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/7092
GitOrigin-RevId: 201ee3ddc205bfc9d55c167e0b70b6606dbe4aa7
2022-11-30 12:12:21 +00:00
Auke Booij
67b922bac1 Avoid GraphQL schema rebuild when changing irrelevant Metadata
This increases the speed of `create_query_collection` and `add_collection_to_allowlist` by a factor ~~10~~ 65, by caching the in-memory GraphQL schema. This speedup also applies more broadly to Metadata changes relating to:
- allowlists
- query collections
- cron triggers
- REST endpoints
- API limits
- metrics config
- GraphQL introspection options
- TLS allow lists
- OpenTelemetry

When is construction of the in-memory GraphQL schema cached between Metadata operations?

Before this PR, **never**! It's rebuilt fully, for every role, on every Metadata operation.

However, there are many Metadata operations that don't influence the GraphQL schema. So we should be caching its construction.

The `Hasura.Incremental` framework allows us to cache such constructions: whenever we have an arrow `Rule m a b`, where `a` is the input to the arrow and `b` the output, we can use the `Inc.cache` combinator to obtain a new arrow which is only re-executed when the input `a` changes in a material way. To test this, `a` needs an `Eq` instance. (Before hasura/graphql-engine-mono#6877, this was a `Cacheable` type class which has now been removed.)

We can't simply apply `Inc.cache` to the "Steps 3 and 4" in `buildSchemaCacheRule`, because the inputs (components of `BuildOutputs` such as `SourceCache`) don't have an `Eq` instance.

So the changes to `buildSchemaCacheRule` restructure the code so that the input to "Step 1", namely the Metadata, can be used as a caching key instead, so that `Inc.cache` can be applied to the whole sequence of steps.

That works to cache construction of the GraphQL schema, but it means that now only those Metadata operations that _don't_ influence any of the products of steps 1-4 can use a cached build of the GraphQL schema. The most important intermediate product is `BuildOutputs`. So now the exercise becomes to minimize the amount of stuff stored in `BuildOutputs`, so that as many Metadata operations as possible can be handled outside of the codepath that produces a GraphQL schema.

Per hasura/graphql-engine-mono#6609, the `BuildOutputs` structure is too big, and stores things unnecessarily. Refer to the PR description there for reasoning - the same logic applies to this PR, and simply goes a few steps further. In doing so, it can benefit from hasura/graphql-engine-mono#6765, which allows us to verify at compile time that certain Schema Cache building steps _don't_ generate "Metadata dependencies". If a certain Metadata dependency is never generated, we don't need to handle that case in `deleteMetadataObject`. Thus such intermediate products don't need to be passed through `resolveDependencies`, and thus they don't need to be stored in `BuildOutputs`, and thus their rebuild won't trigger a GraphQL schema rebuild.

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6613
GitOrigin-RevId: 27d2e69d3461bd4c32f08febef9995c0369fab3a
2022-11-23 07:54:01 +00:00
Auke Booij
05b3a64e8f Clean up Hasura.Prelude a bit
- Remove `onJust` in favor of the more general `for_`
- Remove `withJust` which was used only once
- Remove `hashNub` in favor of `Ord`-based `uniques`
- Simplify some of the implementations in `Hasura.Prelude`
- Add `hlint` hint from `maybe True` to `all`, and `maybe False` to `any`

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6173
GitOrigin-RevId: 2c6ebbe2d04f60071d2a53a2d43c6d62dbc4b84e
2022-10-03 21:50:53 +00:00
Samir Talwar
0869e89faa Remove or ignore all instances of unsafeMkName.
This removes the one remaining instance of `unsafeMkName` in production
code, uses `G.name` where possible in tests, and ignores instances where
it's not possible (such as `instance Arbitrary G.Name`).

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5074
GitOrigin-RevId: d8049edf1f1bc2ef25f34874ef5bd5a5934bd33d
2022-07-18 17:02:15 +00:00
Tom Harding
99f6172d0d Implement HLint suggestions and turn warnings into errors
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4903
GitOrigin-RevId: acab9bbd8373bdf427a80ab1dd73d49ab61996a2
2022-07-01 10:50:33 +00:00
Tom Harding
c90b8c4776 Add forkIO and threadDelay errors to HLint.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4682
GitOrigin-RevId: 35897df6793b41dbf54521a868c2d2daf7e268ea
2022-06-30 09:56:06 +00:00
Brandon Simmons
b704192268 server: GHC 9.2 changes compatible with 8.10 (#3550)
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4841
Co-authored-by: awjchen <13142944+awjchen@users.noreply.github.com>
GitOrigin-RevId: ce47b1290fefb07f3f800c6c62120437c02086e5
2022-06-25 22:09:05 +00:00
Daniel Harvey
88ace749bc server: Fix a bunch of HLint suggestions
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4738
GitOrigin-RevId: d0c0b13ac02ca80e51ae3d582f2e6917f76ad202
2022-06-21 11:12:42 +00:00
Philip Lykke Carlsen
12c3eddef7 Amendments to the hspec testsuite
This PR proposes some changes to the hspec testsuite:

* It amends the framework to make it easier to test from the ghci REPL
* It introduces a new module `Fixture`, distinguished from `Context` by:
   * using a new concept of `SetupAction`s which bundle setup and teardown actions into one abstraction, making test system state setup more concise, modularized and safe (because the fixture know knows about the ordering of setup actions and can do partial rollbacks)
   * somewhat opinionated, elides the `Options` of `Context`, preferring instead that tests that care about stringification of json numbers manage that themselves.

(Note that this PR builds on #4390, so contains some spurious commits which will become irrelevant once that PR is merged)

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4630
GitOrigin-RevId: 619c8d985aed0aa42de31d6f16891d0782f4b4b5
2022-06-08 16:36:50 +00:00
Auke Booij
cda117a4a9 Add unless/when related hints
Example:

```
server/src-lib/Hasura/RQL/DDL/Schema/Table.hs:(200,15)-(205,28): Warning: Use when
Found:
  if tnGQL `elem` ns then
      throw400 RemoteSchemaConflicts
        $ "node " <> tnGQL <> " already exists in current graphql schema"
  else
      pure ()
Perhaps:
  when
    (tnGQL `elem` ns)
    (throw400 RemoteSchemaConflicts
       $ "node " <> tnGQL <> " already exists in current graphql schema")
```

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4570
GitOrigin-RevId: 1b04e5e39d20c99643220154c03dae82a025f0f1
2022-05-27 13:34:42 +00:00
Antoine Leblanc
9308c92e8d Fix a /= [] and add hint.
### Description

Several places in the code used `a /= []`, which is inelegant. To my surprise, hlint did not warn about this, despite the fact that it forces an `Eq` instance on the elements. This PR replaces all occurrences of that pattern with `not (null a)` and adds a lint warning for it.

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4569
GitOrigin-RevId: 6471e75ade9e71e5d583a0dac7815c01870c696b
2022-05-27 12:28:24 +00:00
Robert
cabcc22d5f ci: disable numeric underscore hlint warning
This suggests to e.g. reformat a port number as 8_001 which is
a bad idea.

See e.g. here: https://github.com/hasura/graphql-engine-mono/pull/4082/files

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4140
GitOrigin-RevId: 655d09bd439847e3a5021af787183f95b0b84975
2022-04-01 15:42:08 +00:00
Solomon
c81d9f6962 Ignore Use <$> hlint rule
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3999
GitOrigin-RevId: 07e6653893c6dc146035af947d7a78be7cee6fc1
2022-03-19 00:30:54 +00:00
Antoine Leblanc
f96b889401 Replace all occurrences of mapMaybe id by catMaybes.
### Description

Several libraries define `catMaybes` as `mapMaybe id`. We had it defined in `Data.HashMap.Strict.Extended` already. This small PR also defines it in `Extended` modules for other containers and replaces every occurrence of `mapMaybe id` accordingly.

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3884
GitOrigin-RevId: d222a2ca2f4eb9b725b20450a62a626d3886dbf4
2022-03-03 20:13:10 +00:00
kodiakhq[bot]
1181625173 server: optimize collectTypeDefinitions and refactor
Numbers from CI for the new (currently noisy) `replace_metadata` adhoc benchmark:

    chinook:      0.19s  ->  0.16
    huge_schema: 36.98s  ->  29.89

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3685
GitOrigin-RevId: be79b666858b03e8407c0d89765e9aac0af8d40a
2022-02-24 18:56:22 +00:00
Antoine Leblanc
a023a3259d Prevent uses of unsafeMkName whenever possible.
### Description

This PR is the result of a discussion in #3363. Namely, we would like to remove all uses of `unsafeMkName`, or at the very least document every single one of them, to avoid similar issues. To do so, this PR does the following:
- it adds a hlint suggestion not to use that function:
  - suggestions don't mark the PR as failed, but will be shown at review time
  - it is possible to disable that hint with `{- HLINT ignore myFunction "unsafe" -}`
- wherever possible, it removes uses of `unsafeMkName` in favour of `mkName`
- it adds a comment with a tracking issue for the two remaining uses:
  - #3478
  - #3479

### Remaining work

- discuss whether this hint should make the linter step fail, since the linter step isn't required to merge anyway, and there is a way to disable the hint wherever we think the use of that function is acceptable
- check that none of those uses were load-bearing and result in errors now

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3480
GitOrigin-RevId: 0a7e3e9d1a48185764c04ab61e34b58273af347c
2022-01-27 15:13:37 +00:00
Evie Ciobanu
9297a2e09d server: mssql transactions: rearrange, export less, improve docs, hlint
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3387
GitOrigin-RevId: 904fcf8349ab0626b64ff86cd8b076dd08abff8e
2022-01-19 05:26:49 +00:00
Auke Booij
a7dbe95666 Hlint all Haskell code
So far we've only used `hlint` to lint the OSS code. This moves some things around to lint the Pro code as well.

Note that the CI action only runs `hlint` on Haskell files that are _changed_ by a PR, relative to its merge-base (usually `main`).

As a reminder, you can use the `ignore-server-hlint-checks` label to prevent this from blocking a merge.

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3383
GitOrigin-RevId: d5779c63d780f526a1d58ae4107f0d5262a23ec1
2022-01-18 04:06:15 +00:00