-
Notifications
You must be signed in to change notification settings - Fork 28
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
Introduce deletion-mode for assets #421
Conversation
c3daa07
to
5963dad
Compare
final String keySpace = getKeySpace(); | ||
List<String> statements = | ||
ConfigurationUtils.getList("delete-statements", assetDefinition.getConfig()); | ||
if (statements.isEmpty()) { |
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.
I don't think this is a good idea, the table may have other dependencies like INDEXes it really depends on how you created it (and we have no control over the create-statements)
if there is no statement then we do nothing, like for create-statements
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.
it's not very intuitive.
You set deletion-mode: delete and then you have to provide the keyspace delete statement ?
It's much easier if you just delete it.
From my testing, the keyspace is being deleted even if not empty and it works well.
Do you think an INDEX would make the keyspace deletion fail ?
List<String> statements = | ||
ConfigurationUtils.getList("delete-statements", assetDefinition.getConfig()); | ||
if (statements.isEmpty()) { | ||
statements = List.of("DROP KEYSPACE IF EXISTS %s;".formatted(getKeyspace())); |
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.
also here
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.
if the auto delete command works with tables with indexed (for instance of Vector columns) then I am +1
as an alternative we can fail the deployment here
langstream/langstream-core/src/main/java/ai/langstream/impl/assets/CassandraAssetsProvider.java
Line 30 in 965c78e
public class CassandraAssetsProvider extends AbstractAssetProvider { |
in any case we must add the validation in CassandraAssetsProvider
Same of #418 but for assets.
new field
deletion-mode: none|delete
. Default is noneWhen deletion-mode=delete, the
delete-statements
field is required (list[] string) and all the statements are executed at deletion time.