Skip to content

Conversation

@stepansergeevitch
Copy link
Contributor

@stepansergeevitch stepansergeevitch commented Nov 27, 2024

Check List

  • Tests has been run in packages where changes made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Issue Reference this PR resolves

We've received a support ticket that a customer cannot connect to a stopped engine

Description of Changes Made (if issue reference is not provided)

  • Added engine autostart to ensure we can connect to a stopped engine
  • Added a verbose user message when testing connection fails

@stepansergeevitch stepansergeevitch requested a review from a team as a code owner November 27, 2024 13:12
@vercel
Copy link

vercel bot commented Nov 27, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

8 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-angular-dashboard ⬜️ Ignored (Inspect) Visit Preview Nov 27, 2024 1:12pm
examples-react-d3 ⬜️ Ignored (Inspect) Visit Preview Nov 27, 2024 1:12pm
examples-react-dashboard ⬜️ Ignored (Inspect) Visit Preview Nov 27, 2024 1:12pm
examples-react-data-table ⬜️ Ignored (Inspect) Visit Preview Nov 27, 2024 1:12pm
examples-react-highcharts ⬜️ Ignored (Inspect) Visit Preview Nov 27, 2024 1:12pm
examples-react-material-ui ⬜️ Ignored (Inspect) Visit Preview Nov 27, 2024 1:12pm
examples-react-pivot-table ⬜️ Ignored (Inspect) Visit Preview Nov 27, 2024 1:12pm
examples-vue-query-builder ⬜️ Ignored (Inspect) Visit Preview Nov 27, 2024 1:12pm

@github-actions github-actions bot added the pr:community Contribution from Cube.js community members. label Nov 27, 2024
Copy link
Member

@KSDaemon KSDaemon left a comment

Choose a reason for hiding this comment

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

👍🏻 Great! Thanks for the contribution!

@KSDaemon
Copy link
Member

KSDaemon commented Dec 2, 2024

@stepansergeevitch Do unit tests work for you? I tried the yarn run test and saw that they failed. I wonder if your newly added test works. %)

 FAIL  dist/test/autostart.test.js (7.08 s)
  ● FireboltDriver autostart › calls engine start

    TypeError: Cannot read properties of undefined (reading 'includes')

      85 |
      86 |     const username = getEnv('dbUser', { dataSource });
    > 87 |     const auth = username.includes('@')
         |                           ^
      88 |       ? { username, password: getEnv('dbPass', { dataSource }) }
      89 |       : { client_id: username, client_secret: getEnv('dbPass', { dataSource }) };
      90 |

      at new FireboltDriver (src/FireboltDriver.ts:87:27)
      at test/autostart.test.ts:15:14

@stepansergeevitch
Copy link
Contributor Author

stepansergeevitch commented Dec 2, 2024

@stepansergeevitch Do unit tests work for you? I tried the yarn run test and saw that they failed. I wonder if your newly added test works. %)

 FAIL  dist/test/autostart.test.js (7.08 s)
  ● FireboltDriver autostart › calls engine start

    TypeError: Cannot read properties of undefined (reading 'includes')

      85 |
      86 |     const username = getEnv('dbUser', { dataSource });
    > 87 |     const auth = username.includes('@')
         |                           ^
      88 |       ? { username, password: getEnv('dbPass', { dataSource }) }
      89 |       : { client_id: username, client_secret: getEnv('dbPass', { dataSource }) };
      90 |

      at new FireboltDriver (src/FireboltDriver.ts:87:27)
      at test/autostart.test.ts:15:14

Hey @KSDaemon
This test actually requires you to set the env variables for the driver.
Specifiсally CUBEJS_DB_USER, CUBEJS_DB_PASS, CUBEJS_DB_NAME, CUBEJS_FIREBOLT_ACCOUNT and CUBEJS_FIREBOLT_ENGINE_NAME
So that it's able to connect to the database

@KSDaemon
Copy link
Member

KSDaemon commented Dec 3, 2024

@stepansergeevitch Thnx for the notes! I see... So it's kind of impossible to setup isolated unit/integration tests. Anyway it requires cloud account/connection....

@KSDaemon
Copy link
Member

KSDaemon commented Dec 3, 2024

@stepansergeevitch, Could you please add a console.log message at the beginning of the test suite saying that it is required to set up all those ENVs if some of the required ones are missing? This will help future contributors.

UPD: NVM.

@KSDaemon
Copy link
Member

KSDaemon commented Dec 5, 2024

@stepansergeevitch Once again — thnx for the contribution! Merging! I'll setup CI integration tests later.

@KSDaemon KSDaemon merged commit 8f45afa into cube-js:master Dec 5, 2024
21 checks passed
@KSDaemon
Copy link
Member

KSDaemon commented Dec 5, 2024

@stepansergeevitch Sorry for bothering you again. I created a firebolt.io account, set up everything and tried to run the tests.
But it fails. Can you help me?

If the engine (drivers_test) is running, and I run tests, I get these errors:

 FAIL  dist/test/Firebolt.test.js (11.097 s)
  FireboltDriver
    ✕ query (7636 ms)
    ✕ stream (732 ms)

  ● FireboltDriver › query


    Request failed
    URL: https://my-account.api.us-east-1.app.firebolt.io/?database=UltraFast&output_format=JSON_Compact&engine=drivers_test
    Reason: no instances in engine cluster

    Response status: 502

      264 |     if (this.config.connection.engineName) {
      265 |       const engine = await this.firebolt.resourceManager.engine.getByName(this.config.connection.engineName);
    > 266 |       await engine.startAndWait();
          |       ^
      267 |     }
      268 |   }


  ● FireboltDriver › stream


    Request failed
    URL: https://my-account.api.us-east-1.app.firebolt.io/?database=UltraFast&output_format=JSON_Compact&engine=drivers_test
    Reason: You are attempting to run a query on engine 'drivers_test', but either it does not exist, it is stopped, or you don't have permission to access it.  Make sure the engine is running or try running the query on a different engine.

    Response status: 404

      263 |   private async ensureEngineRunning() {
      264 |     if (this.config.connection.engineName) {
    > 265 |       const engine = await this.firebolt.resourceManager.engine.getByName(this.config.connection.engineName);
          |                      ^
      266 |       await engine.startAndWait();
      267 |     }
      268 |   }

But autostart tests pass:

 PASS  dist/test/autostart.test.js (42.431 s)
  FireboltDriver autostart
    ✓ calls engine start (2 ms)
    ✓ starts the engine after connection (39745 ms)

If the engine is stopped before running tests, I get these errors:

 FAIL  dist/test/Firebolt.test.js (5.294 s)
  FireboltDriver
    ✕ query (2085 ms)
    ✕ stream (739 ms)

  ● FireboltDriver › query


    Request failed
    URL: https://my-accountapi.us-east-1.app.firebolt.io/?database=UltraFast&output_format=JSON_Compact&engine=drivers_test
    Reason: You are attempting to run a query on engine 'drivers_test', but either it does not exist, it is stopped, or you don't have permission to access it.  Make sure the engine is running or try running the query on a different engine.

    Response status: 404

      263 |   private async ensureEngineRunning() {
      264 |     if (this.config.connection.engineName) {
    > 265 |       const engine = await this.firebolt.resourceManager.engine.getByName(this.config.connection.engineName);
          |                      ^
      266 |       await engine.startAndWait();
      267 |     }
      268 |   }


  ● FireboltDriver › stream


    Request failed
    URL: https://my-account.api.us-east-1.app.firebolt.io/?database=UltraFast&output_format=JSON_Compact&engine=drivers_test
    Reason: You are attempting to run a query on engine 'drivers_test', but either it does not exist, it is stopped, or you don't have permission to access it.  Make sure the engine is running or try running the query on a different engine.

    Response status: 404

      263 |   private async ensureEngineRunning() {
      264 |     if (this.config.connection.engineName) {
    > 265 |       const engine = await this.firebolt.resourceManager.engine.getByName(this.config.connection.engineName);
          |                      ^
      266 |       await engine.startAndWait();
      267 |     }
      268 |   }

  console.log
    ApiError:
    Request failed
    URL: https://my-account.api.us-east-1.app.firebolt.io/?database=UltraFast&output_format=JSON_Compact&engine=drivers_test
    Reason: You are attempting to run a query on engine 'drivers_test', but either it does not exist, it is stopped, or you don't have permission to access it.  Make sure the engine is running or try running the query on a different engine.

    Response status: 404


 FAIL  dist/test/autostart.test.js (17.557 s)
  FireboltDriver autostart
    ✓ calls engine start (2 ms)
    ✕ starts the engine after connection (15098 ms)

  ● FireboltDriver autostart › starts the engine after connection


    Request failed
    URL: https://my-accountapi.us-east-1.app.firebolt.io/?database=UltraFast&output_format=JSON_Compact&engine=drivers_test
    Reason: You are attempting to run a query on engine 'drivers_test', but either it does not exist, it is stopped, or you don't have permission to access it.  Make sure the engine is running or try running the query on a different engine.

    Response status: 404

      263 |   private async ensureEngineRunning() {
      264 |     if (this.config.connection.engineName) {
    > 265 |       const engine = await this.firebolt.resourceManager.engine.getByName(this.config.connection.engineName);
          |                      ^
      266 |       await engine.startAndWait();
      267 |     }
      268 |   }

At the same time I see in firebolt.io console, that the engine was started and running.

If I just comment out await this.ensureEngineRunning(); in InitConnection() — queries tests pass:

 PASS  dist/test/Firebolt.test.js
  FireboltDriver
    ✓ query (1678 ms)
    ✓ stream (135 ms)

Can you help me with this?

KSDaemon pushed a commit that referenced this pull request Dec 12, 2024
* automatically start the engine after connection

* add tests

* fix linter

* start engine in the end of the test
KSDaemon added a commit that referenced this pull request Dec 12, 2024
… (#9040)

* automatically start the engine after connection

* add tests

* fix linter

* start engine in the end of the test

Co-authored-by: Stepan Burlakov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:community Contribution from Cube.js community members.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants