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.

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

Fix: Face issue when testing the VulcanSQL integration with Canner Enteprise #185

Merged
merged 2 commits into from
Jun 12, 2023

Conversation

kokokuo
Copy link
Contributor

@kokokuo kokokuo commented Jun 12, 2023

Description

Faced the issues when testing the VulcanSQL Integration with Canner Enterprise:

  1. API Documentation is not consistent for the API prefix when getting the artifacts from Canner Enterprise and merging to one artifact, we need to make the operationId and summary field add the workspace SQL name in specs, then it could solve the solution.

  2. The generated canner profile name in CannerProfileReader is not the same as CannerPersistenceStore, so we need to fix it to the same canner-{workspaceSqlName}.

Issue ticket number

closes N/A

Screenshot for integrating with Canner Enterprise.

We opened a Canner Enterprise of PR testing env and create two workspaces w1 and w2 with the tables:
PR env - w1

PR env - w2

Use the In the VulcanSQL labs and create two profiles canner-w1 and canner-w2 with temporal PAT fo connecting Canner Enterprise by extension-driver-canner through PG Wire Protocol:
截圖 2023-06-12 下午3 44 23

Below, we put the two SQL with API Schema definition to query w1 and run vulcan build for building the SQL and API schema to the artifact for w1
截圖 2023-06-12 下午3 41 29

Upload them to Canner Enterprise, and see the uploaded artifacts in VulcanSQL folder of w1:

截圖 2023-06-12 下午4 00 13
截圖 2023-06-12 下午4 00 18

Do the same steps for w2, put the two SQL with API Schema definition to query w2 and run vulcan build for building the SQL and API

截圖 2023-06-12 下午3 41 49

Upload them to Canner Enterprise, and see the uploaded artifacts in VulcanSQL folder of w2:
截圖 2023-06-12 下午3 39 13
截圖 2023-06-12 下午3 39 02

Now we delete the SQLs and API schemas files, in the lasbs project, and update the vulcan.yaml for artifact options, extensions and profiles like below to use the Canner extension:

HINT: currently, the extension not include Canner PAT authenticator, because it's developing, so we will make auth options keep enabled: false.

....
artifact:
  provider: Canner # use Canner Persistence Store
  serializer: JSON
  # Path to build result
  filePath: fe54258b-da25-408b-a019-efb3b36f1b47 # root path

extensions:
  duckdb: "@vulcan-sql/extension-driver-duckdb"
  # Needed extensions for integrating with Canner Enterprise
  canner-driver: "@vulcan-sql/extension-driver-canner"
  canner-store: "@vulcan-sql/extension-store-canner"

profiles:
  # Specify the Canner profile reader to create profiles with different databases indicating to the workspaces of Canner Enterprise
  - type: Canner # use Canner Profile Reader
    options:
      path: fe54258b-da25-408b-a019-efb3b36f1b47 # root path

Then set some environment variables we need:

MINIO_ACCESS_KEY=<ACCESS-KEY>
MINIO_BUCKET=<BUCKET>
MINIO_PORT=9000
MINIO_SECRET_KEY=<SECRETE-KEY>
MINIO_SSL=false
MINIO_URL=<IP>

PROFILE_CANNER_DRIVER_HOST=<IP>
PROFILE_CANNER_DRIVER_PASSWORD=<PAT>
PROFILE_CANNER_DRIVER_USER=<USER>

Finally, run vulcan serve to fetch the artifacts from Canner Enterprise, and generate Canner Enterprise Profiles settings:

截圖 2023-06-12 下午3 23 20

Open the API documentation URL, we will see the workspace SQL Name in the prefix for the w1 and w2 Data API:

截圖 2023-06-12 下午3 21 39

Send request to /api/w1/activity_logs?operation=VIEW to query and you will see the result:
截圖 2023-06-12 下午3 22 18

Send request to /api/w2/resource_metadata?data_type=DATA_SOURCE_TABLE to query and you will see the result:

截圖 2023-06-12 下午3 22 44

You could also see the logs in VulcanSQL:

截圖 2023-06-12 下午3 23 36

Done :)

Additional Context

  • add the logger.debug could see the workspaces indicator information in CannerProfileReader and CannerPersistenceStore.
  • remove unnecessary @VulcanInternalExtension() in CannerProfileReader and CannerPersistenceStore, because both of them are not internal extensions, thet load by external extension type, so the VulcanInternalExtension won't be used

… to summary and operationId in spec of artifact.

- display indicator files of each workspace when log debug info at "CannerPersistenceStore" and "CannerProfileReader"
- support adding workspace sql name prefix to summary and operationId in spec of artifact in the "CannerPersistenceStore"
@vercel
Copy link

vercel bot commented Jun 12, 2023

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

Name Status Preview Comments Updated (UTC)
vulcan-sql-document ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 12, 2023 7:47am

@kokokuo kokokuo changed the title fix: Face issue when testing the VulcanSQL integration with Canner Enteprise (WIP) fix: Face issue when testing the VulcanSQL integration with Canner Enteprise Jun 12, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jun 12, 2023

Codecov Report

Patch coverage: 92.30% and project coverage change: +0.23 🎉

Comparison is base (7523c68) 91.07% compared to head (2d530cc) 91.30%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #185      +/-   ##
===========================================
+ Coverage    91.07%   91.30%   +0.23%     
===========================================
  Files          328      319       -9     
  Lines         5399     5105     -294     
  Branches       718      677      -41     
===========================================
- Hits          4917     4661     -256     
+ Misses         343      314      -29     
+ Partials       139      130       -9     
Flag Coverage Δ
extension-driver-canner ?
extension-store-canner 98.30% <91.66%> (-0.81%) ⬇️
integration-testing 90.27% <ø> (ø)
serve 87.04% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...on-store-canner/src/lib/canner/persistenceStore.ts 98.14% <90.00%> (-1.86%) ⬇️
...nsion-store-canner/src/lib/canner/profileReader.ts 100.00% <100.00%> (ø)
packages/serve/src/lib/evaluator/evaluator.ts 88.09% <100.00%> (+0.29%) ⬆️

... and 15 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

- add some debug logger for providing information about "Evaluator" and "CannerPersistenceStore"
- fix the non-consistency canner profile name in "CannerProfileReader".
- fix missing the root path when download buffer stream of the artifact
- update th test cases
@kokokuo kokokuo force-pushed the fix/vulcan-integration-faced-issues branch from fca8e87 to 2d530cc Compare June 12, 2023 07:46
@kokokuo kokokuo changed the title (WIP) fix: Face issue when testing the VulcanSQL integration with Canner Enteprise Fix: Face issue when testing the VulcanSQL integration with Canner Enteprise Jun 12, 2023
Copy link
Contributor

@fredalai fredalai left a comment

Choose a reason for hiding this comment

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

LGTM

@kokokuo kokokuo merged commit 4a0e2d2 into develop Jun 12, 2023
@kokokuo kokokuo deleted the fix/vulcan-integration-faced-issues branch June 12, 2023 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants