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

feat: add generating item history using AI #72

Merged
merged 8 commits into from
Jun 17, 2024
Merged

feat: add generating item history using AI #72

merged 8 commits into from
Jun 17, 2024

Conversation

ktos
Copy link
Member

@ktos ktos commented Jun 9, 2024

No description provided.

ktos and others added 3 commits June 14, 2024 12:44
This is based on the Łukasz's prompts implemented in March

Co-Authored-By: Ornez <[email protected]>
Co-Authored-By: Łukasz Chudy <[email protected]>
@ktos ktos marked this pull request as ready for review June 14, 2024 10:58
@ktos
Copy link
Member Author

ktos commented Jun 14, 2024

Generating of item history using AI is now behind a feature flag which must be set in appsettings.json, e.g.:

"FeatureFlags": {
  "UseAI": true
}

@ktos
Copy link
Member Author

ktos commented Jun 14, 2024

DO NOT MERGE YET!

Still - if there is no secrets configuration, the AIProvider cannot be constructed and throws. How should we proceed?

We should not use FeatureManagement in a constructor (as it is async), and we cannot register services based on features? I am not sure.

@purifetchi
Copy link
Collaborator

DO NOT MERGE YET!

Still - if there is no secrets configuration, the AIProvider cannot be constructed and throws. How should we proceed?

We should not use FeatureManagement in a constructor (as it is async), and we cannot register services based on features? I am not sure.

you actually can use featuremanagement outside a service scope: https://stackoverflow.com/questions/61659144/how-to-get-featuremanager-in-configureservices

we'd just have to manually create the feature manager class and pass to it our config file

@ktos
Copy link
Member Author

ktos commented Jun 17, 2024

Ok, so I created a AddFeatureGatedServices() extension to the IServiceCollection and now AI services are registered only if the feature flag in a config file is set. Using serviceProvider, because manually constructing everything was not working properly.

@ktos
Copy link
Member Author

ktos commented Jun 17, 2024

Additionally, codestral suggested me something which may be even better:

public static class FeatureFlagServiceExtension
{
    public static IServiceCollection AddTransientIfFeatureEnabled<TInterface, TImplementation>(this IServiceCollection services, string featureName) where TImplementation : class, TInterface
        => services.AddTransientIfFeatureEnabled(featureName, typeof(TInterface), typeof(TImplementation));

    public static IServiceCollection AddTransientIfFeatureEnabled(this IServiceCollection services, string featureName, Type serviceType, Type implementationType)
    {
        var featureManager = services.BuildServiceProvider().GetRequiredService<IFeatureManagerSnapshot>();

        if (featureManager[featureName].Value)
            return services.AddTransient(serviceType, implementationType);

        return services;
    }

    // Add similar methods for AddScoped and AddSingleton if needed...
}

So it should be used something like:

services.AddSingletonIfFeatureEnabled<IMyService, MyService>("MyFeatureFlag");

Not sure if it is more readable than current version.

@purifetchi
Copy link
Collaborator

Additionally, codestral suggested me something which may be even better:

public static class FeatureFlagServiceExtension
{
    public static IServiceCollection AddTransientIfFeatureEnabled<TInterface, TImplementation>(this IServiceCollection services, string featureName) where TImplementation : class, TInterface
        => services.AddTransientIfFeatureEnabled(featureName, typeof(TInterface), typeof(TImplementation));

    public static IServiceCollection AddTransientIfFeatureEnabled(this IServiceCollection services, string featureName, Type serviceType, Type implementationType)
    {
        var featureManager = services.BuildServiceProvider().GetRequiredService<IFeatureManagerSnapshot>();

        if (featureManager[featureName].Value)
            return services.AddTransient(serviceType, implementationType);

        return services;
    }

    // Add similar methods for AddScoped and AddSingleton if needed...
}

So it should be used something like:

services.AddSingletonIfFeatureEnabled<IMyService, MyService>("MyFeatureFlag");

Not sure if it is more readable than current version.

i actually like this one better!

@purifetchi purifetchi merged commit c328481 into main Jun 17, 2024
1 check passed
@Xyloo Xyloo deleted the item-history branch October 22, 2024 16:51
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