Aircloak / aircloak

This repository contains the Aircloak Air frontend as well as the code for our Cloak query and anonymization platform
2 stars 0 forks source link

Make live editor feedback less jittery #4831

Closed edongashi closed 3 years ago

edongashi commented 3 years ago

Fixes #4830. Leading edge does not make sense IMO because we can't expect any meaningful feedback on the first keystroke.

cristianberneanu commented 3 years ago

Leading edge does not make sense IMO because we can't expect any meaningful feedback on the first keystroke.

What about a copy-paste operation?

edongashi commented 3 years ago

What about a copy-paste operation?

Good point. In that case you'd get a 1 second delay before requesting feedback. The downside is that if you start from an empty editor you will see "expected select, show, or explain" on the very first keystroke.

I'm fine with whichever approach that fits the more common use case...

aircloak-robot commented 3 years ago

Standard tests have passed 👏

gampleman commented 3 years ago

I don't think this is the right solution. Editor feedback should be as fast as possible, delaying it by a second is the opposite of what you want.

I'd be more in favour of showing the "Loading" message after a substantial delay, but sending the request right away.

edongashi commented 3 years ago

I'd be more in favour of showing the "Loading" message after a substantial delay, but sending the request right away.

You mean show Loading... when it's loading for more than X ms like react suspense does?

That could work. I'll think about how to implement it.

edongashi commented 3 years ago

Debounced the loading indicator. It should now display once you finished typing and if results take more than 250 ms to come back. Please give it a look because the exact feel is hard to balance.

aircloak-robot commented 3 years ago

air_test job errored 💔

You can see the full build log by running: ci/production.sh build_log pr 4831 air_test

You can restart the build by running: ci/production.sh force_build pr 4831 air_test

You can start the remote console by running: ci/production.sh remote_console pr 4831 air

Log tail:

  lib/air/service/ldap/filter_parser.ex:14: Air.Service.LDAP.FilterParser.parse/1

warning: documentation references "(Map.t()) :: {:ok, Air.Schemas.User.t()} | :error" but it is undefined or private
  lib/air/service/user.ex:427: Air.Service.User.add_preconfigured_user/1

warning: documentation references "() | nil) :: Map.t()" but it is undefined or private
  lib/air/service/user.ex:387: Air.Service.User.number_format_settings/1

warning: documentation references "(), Map.t())" but it is undefined or private
  lib/air/service/user.ex:69: Air.Service.User.reset_password/2

warning: documentation references "(), Air.Service.DataSource.t()) :: [Air.Schemas.View.t()" but it is undefined or private
  lib/air/service/view.ex:46: Air.Service.View.all/2

warning: documentation references "(
  Air.Schemas.User.t(),
  Air.Service.DataSource.t(),
  String.t(),
  String.t(),
  String.t() | nil,
  revalidation_timeout: non_neg_integer(),
  skip_revalidation: boolean()
) :: {:ok, Air.Schemas.View.t()} | {:error, Ecto.Changeset.t()}" but it is undefined or private
  lib/air/service/view.ex:58: Air.Service.View.create/6

warning: documentation references "(nil | Calendar.date_time()) :: String.t()" but it is undefined or private
  lib/air/utils/date_time.ex:6: Air.Utils.DateTime.time_ago/1

warning: documentation references "(
  pid(),
  String.t(),
  String.t(),
  String.t(),
  String.t(),
  parameters(),
  Air.Service.View.view_map(),
  Air.Service.AnalystTable.analyst_table_map()
) :: :ok | {:error, any()}" but it is undefined or private
  lib/air_web/socket/cloak/main_channel.ex:24: AirWeb.Socket.Cloak.MainChannel.run_query/8

warning: documentation references "([Map.t()]) :: :ok" but it is undefined or private
  lib/air_web/socket/frontend/cloak_stats_channel.ex:9: AirWeb.Socket.Frontend.CloakStatsChannel.broadcast_cloak_stats/1

warning: documentation references "([Map.t()]) :: [Map.t()]" but it is undefined or private
  lib/air_web/views/view_helpers.ex:55: AirWeb.ViewHelpers.order_by_id/1

warning: documentation references "()]) :: [Map.t()" but it is undefined or private
  lib/air_web/views/view_helpers.ex:55: AirWeb.ViewHelpers.order_by_id/1

warning: documentation references file "elixir-lang.org/" but it does not exist
  README.md

warning: documentation references file "../macos_docker.md" but it does not exist
  README.md

View "epub" docs at "doc/air.epub"
=> 9 sec

aircloak_ci: `/home/ci/.aircloak_ci/data/cache/builds/pr-4831/src/air/ci/container.sh run_in_container aircloak_ci_yqrTxKW2PZ3m4ktDDhX6XQ make eslint`
cd assets && yarn lint
yarn run v1.22.5
warning package.json: No license field
$ eslint --max-warnings 0 js test && prettier --version && prettier --no-config -c css/**.css "../*.md" "../priv/**/*.md"
2.1.2
Checking formatting...
All matched files use Prettier code style!
Done in 10.36s.
=> 12 sec

aircloak_ci: `/home/ci/.aircloak_ci/data/cache/builds/pr-4831/src/air/ci/container.sh run_in_container aircloak_ci_yqrTxKW2PZ3m4ktDDhX6XQ make flow`
cd assets && node_modules/.bin/flow check
Error ------------------------------------------------------------------------------------------ js/queries/root.js:5:15

Importing a type from an untyped module makes it `any` and is not safe! Did you mean to add `// @flow` to the top of
`lodash`? [untyped-type-import]

   5| import type { DebouncedFunc } from "lodash";
                    ^^^^^^^^^^^^^

Error ---------------------------------------------------------------------------------------- js/queries/root.js:144:24

Cannot use `DebouncedFunc` as a type because it is an `any`-typed value. Type `DebouncedFunc` properly, so it is no
longer `any`-typed, to use it as an annotation. [value-as-type]

   144|   setTypeCheckLoading: DebouncedFunc<() => void> = debounce(
                               ^^^^^^^^^^^^^

Found 2 errors
Makefile:65: recipe for target 'flow' failed
make: *** [flow] Error 2
=> 74 sec

aircloak_ci: error: 
error running `/home/ci/.aircloak_ci/data/cache/builds/pr-4831/src/air/ci/container.sh run_in_container aircloak_ci_yqrTxKW2PZ3m4ktDDhX6XQ make flow`
aircloak_ci: result: `error`
aircloak_ci: finished in 6:08 min
aircloak-robot commented 3 years ago

integration_tests_test job errored 😞

You can see the full build log by running: ci/production.sh build_log pr 4831 integration_tests_test

You can restart the build by running: ci/production.sh force_build pr 4831 integration_tests_test

You can start the remote console by running: ci/production.sh remote_console pr 4831 integration_tests

Log tail:

12:27:48.877 [info]  == Running 20180216112239 Central.Repo.Migrations.DropAirCloakAndUsageInfoTables.up/0 forward

12:27:48.877 [info]  drop table cloaks

12:27:48.879 [info]  drop table usage_info

12:27:48.881 [info]  drop table airs

12:27:48.883 [info]  == Migrated 20180216112239 in 0.0s

12:27:48.888 [info]  == Running 20180302151358 Central.Repo.Migrations.CreateLicenses.change/0 forward

12:27:48.888 [info]  create table licenses

12:27:48.891 [info]  == Migrated 20180302151358 in 0.0s

12:27:48.895 [info]  == Running 20180305121014 Central.Repo.Migrations.AddRenewalToLicenses.change/0 forward

12:27:48.895 [info]  alter table licenses

12:27:48.896 [info]  == Migrated 20180305121014 in 0.0s

12:27:48.900 [info]  == Running 20180307123951 Central.Repo.Migrations.AddRevokedToLicenses.change/0 forward

12:27:48.900 [info]  alter table licenses

12:27:48.901 [info]  == Migrated 20180307123951 in 0.0s

12:27:48.904 [info]  == Running 20180423153920 Central.Repo.Migrations.CascadeDeleteLicenses.up/0 forward

12:27:48.904 [info]  execute "ALTER TABLE licenses DROP CONSTRAINT licenses_customer_id_fkey"

12:27:48.905 [info]  alter table licenses

12:27:48.907 [info]  == Migrated 20180423153920 in 0.0s

12:27:48.911 [info]  == Running 20180508143800 Central.Repo.Migrations.DeleteQueriesTable.up/0 forward

12:27:48.911 [info]  drop table queries

12:27:48.913 [info]  == Migrated 20180508143800 in 0.0s

12:27:48.918 [info]  == Running 20180820131126 Central.Repo.Migrations.AddFeaturesToLicenses.change/0 forward

12:27:48.918 [info]  alter table licenses

12:27:48.922 [info]  == Migrated 20180820131126 in 0.0s

12:27:48.926 [info]  == Running 20190423124742 Central.Repo.Migrations.MakeLoginCaseInsensitive.up/0 forward

12:27:48.926 [info]  drop index users_email_index

12:27:48.927 [info]  execute "CREATE UNIQUE INDEX users_email_index ON users (lower(email))"

12:27:48.928 [info]  == Migrated 20190423124742 in 0.0s

12:27:48.932 [info]  == Running 20200108134139 Central.Repo.Migrations.RemoveAirpcsAndCustomerExports.up/0 forward

12:27:48.932 [info]  drop table air_rpcs

12:27:48.934 [info]  drop table customer_exports

12:27:48.936 [info]  == Migrated 20200108134139 in 0.0s
=> 6 sec

aircloak_ci: `/home/ci/.aircloak_ci/data/cache/builds/pr-4831/src/integration_tests/ci/container.sh run_in_container aircloak_ci_shHtmwKvbGezzhMkDrtJxw mix test`

12:27:51.904 [info]  Compiling auto-completions
............................................................................

  1) test analyst table name can't be the same as a name of an existing view (IntegrationTest.AnalystTableTest)
     test/analyst_table_test.exs:235
     ** (MatchError) no match of right hand side value: {:error, #Ecto.Changeset<action: :insert, changes: %{comment: "view comment", data_source_id: 1, name: "table_37379", sql: "select user_id, name from users", user_id: 40}, errors: [sql: {"The view cannot be saved because no cloak is currently available for the given data source. Please contact your administrator.", []}], data: #Air.Schemas.View<>, valid?: false>}
     code: {:ok, _} =
     stacktrace:
       test/analyst_table_test.exs:238: (test)

  2) test converting analyst table to view (IntegrationTest.AnalystTableTest)
     test/analyst_table_test.exs:197
     ** (MatchError) no match of right hand side value: {:error, #Ecto.Changeset<action: :insert, changes: %{comment: "comment", data_source_id: 1, name: "table_37635", sql: "select user_id, name from users", user_id: 40}, errors: [sql: {"The table cannot be saved because no cloak is currently available for the given data source. Please contact your administrator.", []}], data: #Air.Schemas.AnalystTable<>, valid?: false>}
     code: {:ok, table} = create_table(context.user, name, "select user_id, name from users", "comment")
     stacktrace:
       test/analyst_table_test.exs:200: (test)

..................................................................

Finished in 68.3 seconds
144 tests, 2 failures

Randomized with seed 7911
[os_mon] memory supervisor port (memsup): Erlang has closed
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
=> 74 sec

aircloak_ci: error: error running `/home/ci/.aircloak_ci/data/cache/builds/pr-4831/src/integration_tests/ci/container.sh run_in_container aircloak_ci_shHtmwKvbGezzhMkDrtJxw mix test`
aircloak_ci: result: `error`
aircloak_ci: finished in 2:10 min
aircloak-robot commented 3 years ago

air_test job errored 🔥

You can see the full build log by running: ci/production.sh build_log pr 4831 air_test

You can restart the build by running: ci/production.sh force_build pr 4831 air_test

You can start the remote console by running: ci/production.sh remote_console pr 4831 air

Log tail:


warning: documentation references "()) :: {:ok, :eldap.filter()" but it is undefined or private
  lib/air/service/ldap/filter_parser.ex:14: Air.Service.LDAP.FilterParser.parse/1

warning: documentation references "(Map.t()) :: {:ok, Air.Schemas.User.t()} | :error" but it is undefined or private
  lib/air/service/user.ex:427: Air.Service.User.add_preconfigured_user/1

warning: documentation references "() | nil) :: Map.t()" but it is undefined or private
  lib/air/service/user.ex:387: Air.Service.User.number_format_settings/1

warning: documentation references "(), Map.t())" but it is undefined or private
  lib/air/service/user.ex:69: Air.Service.User.reset_password/2

warning: documentation references "(), Air.Service.DataSource.t()) :: [Air.Schemas.View.t()" but it is undefined or private
  lib/air/service/view.ex:46: Air.Service.View.all/2

warning: documentation references "(
  Air.Schemas.User.t(),
  Air.Service.DataSource.t(),
  String.t(),
  String.t(),
  String.t() | nil,
  revalidation_timeout: non_neg_integer(),
  skip_revalidation: boolean()
) :: {:ok, Air.Schemas.View.t()} | {:error, Ecto.Changeset.t()}" but it is undefined or private
  lib/air/service/view.ex:58: Air.Service.View.create/6

warning: documentation references "(nil | Calendar.date_time()) :: String.t()" but it is undefined or private
  lib/air/utils/date_time.ex:6: Air.Utils.DateTime.time_ago/1

warning: documentation references "(
  pid(),
  String.t(),
  String.t(),
  String.t(),
  String.t(),
  parameters(),
  Air.Service.View.view_map(),
  Air.Service.AnalystTable.analyst_table_map()
) :: :ok | {:error, any()}" but it is undefined or private
  lib/air_web/socket/cloak/main_channel.ex:24: AirWeb.Socket.Cloak.MainChannel.run_query/8

warning: documentation references "([Map.t()]) :: :ok" but it is undefined or private
  lib/air_web/socket/frontend/cloak_stats_channel.ex:9: AirWeb.Socket.Frontend.CloakStatsChannel.broadcast_cloak_stats/1

warning: documentation references "([Map.t()]) :: [Map.t()]" but it is undefined or private
  lib/air_web/views/view_helpers.ex:55: AirWeb.ViewHelpers.order_by_id/1

warning: documentation references "()]) :: [Map.t()" but it is undefined or private
  lib/air_web/views/view_helpers.ex:55: AirWeb.ViewHelpers.order_by_id/1

warning: documentation references file "elixir-lang.org/" but it does not exist
  README.md

warning: documentation references file "../macos_docker.md" but it does not exist
  README.md

View "epub" docs at "doc/air.epub"
=> 10 sec

aircloak_ci: `/home/ci/.aircloak_ci/data/cache/builds/pr-4831/src/air/ci/container.sh run_in_container aircloak_ci_U5Y_lmmXXQe60PCtBSHXAA make eslint`
cd assets && yarn lint
yarn run v1.22.5
warning package.json: No license field
$ eslint --max-warnings 0 js test && prettier --version && prettier --no-config -c css/**.css "../*.md" "../priv/**/*.md"
2.1.2
Checking formatting...
All matched files use Prettier code style!
Done in 10.50s.
=> 11 sec

aircloak_ci: `/home/ci/.aircloak_ci/data/cache/builds/pr-4831/src/air/ci/container.sh run_in_container aircloak_ci_U5Y_lmmXXQe60PCtBSHXAA make flow`
cd assets && node_modules/.bin/flow check
Error ----------------------------------------------------------------------------------------- js/queries/root.js:143:3

Cannot build a typed interface for this module. You should annotate the exports of this module with types. Missing type
annotation at property `setTypeCheckLoading`: [signature-verification-failure]

          v------------------------------
   143|   setTypeCheckLoading = debounce(
   144|     () => {
   145|       this.setState({ annotations: "loading" });
   146|     },
   147|     250,
   148|     { trailing: true }
   149|   );
          -^

Found 1 error
Makefile:65: recipe for target 'flow' failed
make: *** [flow] Error 2
=> 76 sec

aircloak_ci: error: 
error running `/home/ci/.aircloak_ci/data/cache/builds/pr-4831/src/air/ci/container.sh run_in_container aircloak_ci_U5Y_lmmXXQe60PCtBSHXAA make flow`
aircloak_ci: finished in 5:23 min
aircloak_ci: result: `error`
aircloak-robot commented 3 years ago

Standard tests have passed ❤️

aircloak-robot commented 3 years ago

Pull request can be merged 🎉