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

Peter/fix schema multitenant aggregates wrong tenant #491

Conversation

pshoukry
Copy link
Contributor

@pshoukry pshoukry commented Feb 26, 2025

Contributor checklist

  • Bug fixes include regression tests
  • Features include unit/acceptance tests

@pshoukry
Copy link
Contributor Author

@zachdaniel sorry this is the test with the 2 loads. It fails cause its trying to load from the wrong database.

@zachdaniel zachdaniel merged commit f70cd70 into ash-project:main Feb 27, 2025
51 of 54 checks passed
@pshoukry pshoukry deleted the peter/fix_schema_multitenant_aggregates_wrong_tenant branch February 27, 2025 17:15
@zachdaniel
Copy link
Contributor

So the issue in this case was that the tenant option was not being set, but you should have gotten a better error. Additionally, we should have been able to infer the tenant from the record. I'm adding this behavior to ash core.

@pshoukry
Copy link
Contributor Author

@zachdaniel thank you for looking in to it

@zachdaniel
Copy link
Contributor

Actually in this case we can't infer the tenant, because the org resource is not multitenant. Either way, an error will make it clearer in the future.

@pshoukry
Copy link
Contributor Author

@zachdaniel sounds great will appreciate if you can mention me in the related PR so I can understand the issue more

@zachdaniel
Copy link
Contributor

Its on this commit in ash 7909909a16af47a22a7e3cdbcd1a8244cc4ef499

@pshoukry
Copy link
Contributor Author

🙇‍♂️ thank you!

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.

2 participants