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

Support remote config from extraction pipelines [DEGR-2723] #63

Merged
merged 30 commits into from
Nov 28, 2023

Conversation

kbrattli
Copy link
Contributor

@kbrattli kbrattli commented Aug 8, 2023

Symmtry (TESTED AND WORKS): https://github.com/cognitedata/symmetry-connector-dotnet/pull/23
SimConnect (TESTED AND WORKS): https://github.com/cognitedata/simconnect-dotnet/pull/137
DWSIM (TESTED AND WORKS): https://github.com/cognitedata/dwsim-connector-dotnet/pull/34
PetroSim (TESTED AND WORKS): https://github.com/cognitedata/petrosim-connector-dotnet/pull/30


https://cognitedata.atlassian.net/browse/DEGR-2723

ConnectorBase.cs
This allows us to check for if the Remote Config from fusion.cognite has been updated. Every connector has to be updated but it is small change, simply do this:

public class SymmetryConnector : ConnectorBase<FullConfig> //------- Pay attention here
{

    private readonly ModelLibrary _modelLibrary;

    private readonly ConfigurationLibrary _configurationLibrary;

    private readonly FullConfig _config;
    private readonly ILogger<SymmetryConnector> _logger;

    private readonly SymmetryClient _symmetry;

    private readonly SimulationRunner _simulationRunner;

    private readonly SimulationScheduler _simulationScheduler;

    private readonly ExtractionPipeline _pipeline;

    private readonly string _version;

    public SymmetryConnector(
         CogniteDestination cdf,
         FullConfig config,
        ModelLibrary modelLibrary,
        ConfigurationLibrary configurationLibrary,
        SymmetryClient symmetry,
        SimulationScheduler scheduler,
        SimulationRunner runner,
        ExtractionPipeline pipeline,
        ILogger<SymmetryConnector> logger,
        RemoteConfigManager<FullConfig> remoteConfigManager) //------ Pay attention here
         : base(cdf, config.Connector, new List<SimulatorConfig> { config.Simulator }, logger, remoteConfigManager) //--- Pay attention here
    {

Then add this in the run(): taskList.Add(CheckForNewConfig(linkedToken));. Important to check that every task uses linkedToken and not token in Run(), if one of them uses token, CheckForNewConfig will not work.

Also remember to change ConnectorRuntime.cs in every connector. Look here for guidance: https://github.com/cognitedata/symmetry-connector-dotnet/pull/23. The important idea is that the NewConfigException needs to propagate the whole way up to get catched and then can restart the connector. So if you add a new catch for general exception, it is important that you write: catch (Exception e) when (e is not NewConfigDetected)


ExtractionPipeline.cs
This helper function first extracts the local config, then it will retrieve the "minimum config" aka the "cognite and type" part of the cognite. Then if type is remote, addRemoteConfig will get the FullConfig from Monitor Extraction Pipelines. To do this it has to be linked to a Extraction Pipeline with for example:

type: remote
cognite:
  extraction-pipeline:
    pipeline-id: symmetry-extraction-pipeline-kenneth

if type is local, then it will just get the local config instead.

This function has to be used in every ConnectorRuntime.cs see below for example, also add the exception handling.

@kbrattli kbrattli requested a review from a team as a code owner August 8, 2023 13:45
@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Merging #63 (9b3042d) into main (dcdcc2b) will decrease coverage by 0.60%.
The diff coverage is 8.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #63      +/-   ##
==========================================
- Coverage   71.93%   71.33%   -0.60%     
==========================================
  Files          28       28              
  Lines        3976     4012      +36     
  Branches      451      454       +3     
==========================================
+ Hits         2860     2862       +2     
- Misses        914      948      +34     
  Partials      202      202              
Files Coverage Δ
Cognite.Simulator.Utils/ConnectorBase.cs 58.38% <17.64%> (-5.07%) ⬇️
Cognite.Simulator.Utils/ExtractionPipeline.cs 47.40% <0.00%> (-7.08%) ⬇️

@kbrattli kbrattli changed the title Helper function for adding remote config [DEGR-2723] check for new remote config + helper function for adding remote config [DEGR-2723] Aug 11, 2023
@sagar-thalwar
Copy link
Collaborator

sagar-thalwar commented Nov 20, 2023

The version needs to be updated - maybe a major update?

@@ -1 +1 @@
1.0.0-alpha-013
1.0.0-alpha-014
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1.0.0-alpha-014
2.0.0-alpha-001

@polomani polomani changed the title check for new remote config + helper function for adding remote config [DEGR-2723] Support remote config from extraction pipelines [DEGR-2723] Nov 28, 2023
@abdullah-cognite abdullah-cognite merged commit f5a140a into main Nov 28, 2023
2 checks passed
@abdullah-cognite abdullah-cognite deleted the remote-config-utils branch November 28, 2023 14:06
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.

4 participants