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.

Sign up for GitHub

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

Jump to bottom

docs(recipes): Using Different Schemas for Tenants #3444

Merged
merged 10 commits into from Oct 25, 2021
Merged

docs(recipes): Using Different Schemas for Tenants #3444

merged 10 commits into from Oct 25, 2021

Conversation

rchkv
Copy link
Contributor

@rchkv rchkv commented Sep 17, 2021 •
edited

@rchkv rchkv added the docs Issues that require a documentation improvement label Sep 17, 2021
@rchkv rchkv self-assigned this Sep 17, 2021
@rchkv rchkv requested a review from a team as a code owner September 17, 2021 08:01
hassankhan
hassankhan reviewed Sep 23, 2021
View reviewed changes
Copy link
Contributor

@hassankhan hassankhan left a comment

Choose a reason for hiding this comment

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

Minor nits, great work @rchkv 👍

docs/content/Examples-Tutorials-Recipes/Using-different-schemas-for-tenants.md Outdated Show resolved Hide resolved
docs/content/Examples-Tutorials-Recipes/Using-different-schemas-for-tenants.md Outdated Show resolved Hide resolved
docs/content/Examples-Tutorials-Recipes/Using-different-schemas-for-tenants.md Outdated Show resolved Hide resolved
docs/content/Examples-Tutorials-Recipes/Using-different-schemas-for-tenants.md Outdated Show resolved Hide resolved
rchkv and others added 2 commits September 23, 2021 21:42
@rchkv @hassankhan
Apply suggestions from code review
7448557 
Co-authored-by: Hassan Khan <hassan@cube.dev>
@rchkv
add folders sctructure
41bdf6e
@rpaik
Copy link
Contributor

rpaik commented Sep 24, 2021

Thanks @rchkv, found a couple of minor items 😄

@rchkv @rpaik
Apply suggestions from code review
e52ce7d 
Co-authored-by: Ray Paik <ray@cube.dev>
@igorlukanin
Copy link
Member

Looks good to me with a few things I'd like to mention:

  • I've tuned the text a little bit
  • I've also changed the title and updated file names + locations accordingly. Please check I didn't mess things up 😬
  • I think we also need to define scheduledRefreshContexts because currently a warning + an error message from the refresh worker are printed in the console. It's not a multitenancy best practice 😄
  • I also saw this warning in the console: "(node:1) DeprecationWarning: Using absolute import with @cubejs-backend/server-core is deprecated". It might be unrelated to this recipe (or maybe I just have an old Cube version) but if it's what we currently log with the most recent Cube version, it's worth an issue on GitHub or a ping to the engineering team

@igorlukanin
Copy link
Member

All in all, great job, @rchkv!

@igorlukanin igorlukanin merged commit c802718 into master Oct 25, 2021
@igorlukanin igorlukanin deleted the recipes/repositoryFactory branch October 25, 2021 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hassankhan hassankhan hassankhan left review comments

@rpaik rpaik rpaik left review comments

@igorlukanin igorlukanin Awaiting requested review from igorlukanin

@adnanrahic adnanrahic Awaiting requested review from adnanrahic

Assignees

@rchkv rchkv

Labels
docs Issues that require a documentation improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@rchkv @rpaik @igorlukanin @hassankhan