-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Phoenix dex fix #6281
Phoenix dex fix #6281
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to merge when CI passes ✔️
I've made a couple small adjustments that should also be replicated once we remove columns from other base spells.
- rename the model to
_base_trades
- add the
check_columns_solana_dex_trades
schema test - add a compatibility view at the old model name, @aalan3 notified me these solana dex models do see some usage on the base level so we try and not break those user queries.
good callouts and foresight! I'll copy the same pattern for meteora/goose when I get to it later today/tomorrow |
redo phoenix fix without seed