carbon-aware-sdk icon indicating copy to clipboard operation
carbon-aware-sdk copied to clipboard

Implement Data Source Registration per ADR-0006

Open bderusha opened this issue 1 year ago • 6 comments

Pull Request

Issue Number: #348

Summary

Loads data sources declared in the .csproj file and configured using the standard configuration dynamically rather than through the hardcoded registration project.

There are many files with tiny, ancillary changes necessary to plumb everything together this way. However the main changes are to the data source files:

  • https://github.com/Green-Software-Foundation/carbon-aware-sdk/pull/366/files#diff-26feb5adea143f40281734e48dda7cd2d24e02a97d601b4863e008d2c319eec7
  • https://github.com/Green-Software-Foundation/carbon-aware-sdk/pull/366/files#diff-ae46f959a52bb012408c419a05dab7910549f3649567fc9950315499c83a2f1b
  • https://github.com/Green-Software-Foundation/carbon-aware-sdk/pull/366/files#diff-6df1e204d60efedcc6ebdffc387fb0d4a9dd02d5ce9dee146adfc995fc507863
  • https://github.com/Green-Software-Foundation/carbon-aware-sdk/pull/366/files#diff-ae7e9ee6050b15aea986d9af6c194a29b5dc427752e96be23aeb0ec2fbb4f1c1

And the Configuration class And the ServiceCollectionExtension

Changes

  • Dynamic loading support
  • All data sources updated to match the interface
  • Updated documentation

Checklist

  • [x] Local Tests Passing?
  • [x] CICD and Pipeline Tests Passing?
  • [x] Added any new Tests?
  • [x] Documentation Updates Made?
  • [x] Are there any API Changes? If yes, please describe below.
  • [x] This is not a breaking change. If it is, please describe it below.

Are there API Changes?

No

Is this a breaking change?

Yes, projects using the SDK Library now must declare data source packages explicitly.

Anything else?

Other comments, collaborators, etc.

Please follow GitHub's suggested syntax to link Pull Requests to Issues via keywords

This PR Closes #348

bderusha avatar Jun 26 '23 14:06 bderusha

Codecov Report

Merging #366 (ac5aae6) into dev (ad44005) will increase coverage by 5.35%. Report is 7 commits behind head on dev. The diff coverage is 65.78%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #366      +/-   ##
==========================================
+ Coverage   74.21%   79.56%   +5.34%     
==========================================
  Files          77       75       -2     
  Lines        2637     2701      +64     
  Branches      266      268       +2     
==========================================
+ Hits         1957     2149     +192     
+ Misses        598      457     -141     
- Partials       82       95      +13     
Files Changed Coverage Δ
src/CarbonAware.WebApi/src/Program.cs 80.26% <ø> (ø)
src/CarbonAware/src/Interfaces/IDataSource.cs 0.00% <0.00%> (ø)
...psFree/mock/ElectricityMapsFreeDataSourceMocker.cs 95.12% <33.33%> (ø)
...s.ElectricityMaps/src/ElectricityMapsDataSource.cs 61.31% <50.00%> (-2.77%) :arrow_down:
...icityMapsFree/src/ElectricityMapsFreeDataSource.cs 63.63% <50.00%> (+63.63%) :arrow_up:
...are.DataSources.WattTime/src/WattTimeDataSource.cs 88.53% <74.28%> (-4.09%) :arrow_down:
...ware/src/Configuration/DataSourcesConfiguration.cs 89.65% <75.00%> (-10.35%) :arrow_down:
...ware/src/Extensions/ServiceCollectionExtensions.cs 76.19% <76.19%> (ø)
...CarbonAware.DataSources.Json/src/JsonDataSource.cs 80.76% <84.61%> (+0.76%) :arrow_up:
...onfiguration/DataSourcesConfigurationExtensions.cs 83.33% <100.00%> (ø)
... and 1 more

... and 6 files with indirect coverage changes

codecov-commenter avatar Jun 29 '23 13:06 codecov-commenter

From #398 - noted for review, @Willmish to sync with @bderusha . Will reach out to you for availability

Willmish avatar Sep 26 '23 07:09 Willmish

@vaughanknight would you be able to do an initial review of this?

Willmish avatar Oct 13 '23 13:10 Willmish

@vaughanknight to set up time with @bderusha to walk throughi n realtime to get this accelerated

vaughanknight avatar Nov 07 '23 08:11 vaughanknight

From #417: @vaughanknight any updates on this?

Willmish avatar Nov 21 '23 08:11 Willmish

@danuw @Willmish @vaughanknight Anything I can do to help land this PR?

bderusha avatar Jan 29 '24 16:01 bderusha

@bderusha , I can see there are some outstanding conflicts here - are you able to help update and see how we can progress please?

danuw avatar May 14 '24 07:05 danuw

@danuw Is this PR something the community is ready to review and merge?

If yes, and we are going to prioritize this PR, I can prioritize making the required updates (both to current conflicts and any feedback).

bderusha avatar May 15 '24 13:05 bderusha