Skip to content

Conversation

to11mtm
Copy link
Member

@to11mtm to11mtm commented Aug 16, 2025

Fixes #

Changes

Please provide a brief description of the changes here.

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

Latest dev Benchmarks

Include data from the relevant benchmark prior to this change here.

This PR's Benchmarks

Include data from after this change here.

@to11mtm to11mtm changed the title update to 6.0 rc3 update to 6.0 Aug 16, 2025
Copy link
Member Author

@to11mtm to11mtm left a comment

Choose a reason for hiding this comment

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

Left some comments, a couple questions do come up...

using Akka.Persistence.Sql.Db;
using Akka.Persistence.Sql.Journal.Types;
using LinqToDB;
using LinqToDB.Async;
Copy link
Member Author

Choose a reason for hiding this comment

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

Things got moved, now this namespace is a thing for ToListAsync/ToDictionaryAsync etc.

Comment on lines +68 to +69
ProviderType.MySqlOfficial => ProviderName.MySql80MySqlData,//.MySqlOfficial,
ProviderType.MySqlConnector => ProviderName.MySql80MySqlConnector,
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we add ProviderType changes here for the Tag Table migration Big difference is things are now a bit more explicit for MySQL versions...

Not sure if this implies config updates required (We could always try to translate if needed I guess?

public override string ConnectionString => _connectionStringBuilder.ToString();

public override string ProviderName => LinqToDB.ProviderName.MySqlOfficial;
public override string ProviderName => LinqToDB.ProviderName.MySql80MySqlConnector;//.MySqlOfficial;
Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly should actually be MySql80MySqlData ?


public AkkaDataConnection Clone()
=> new(_providerName, (DataConnection)_connection.Clone());
=> throw new NotSupportedException();//_providerName, (DataConnection)_connection.Clone());
Copy link
Member Author

@to11mtm to11mtm Aug 16, 2025

Choose a reason for hiding this comment

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

We have twothree options here...

Option 1 - Take what I did here and treat it as Not Supported.

Option 2 - Have AkkaDataConnection take in the actual connection string as well, and then just have Clone() use that

Option 3 - Just get rid of this method; -should- be a compile time break (bad) but should show up right away in most sane CI setups.

(If we go with 1 or 2, we'll still need to mark this as obsolete via attr)

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.

1 participant