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

Consolidate providers into one folder #44

Closed
fershad opened this issue Jul 22, 2022 · 5 comments
Closed

Consolidate providers into one folder #44

fershad opened this issue Jul 22, 2022 · 5 comments
Assignees

Comments

@fershad
Copy link
Collaborator

fershad commented Jul 22, 2022

Current state
Providers are all kept in the root of the project.

Desired future state
Providers are within kept within the same folder /providers.

Why?
This will make it easier for people to identify and contribute new providers. It will also make the project cleaner to navigate as more providers are added.

@rossf7
Copy link
Contributor

rossf7 commented Aug 1, 2022

@fershad Thanks this will clean up the structure a lot.

I think we should use /pkg/providers as the package name. The pkg follows the Go convention for library code and aligns with cmd that we use for the CLI.

@rossf7
Copy link
Contributor

rossf7 commented Aug 1, 2022

I've also been thinking that we need to change the interface we use for the providers. Currently it returns the metric value but there is no context about what type of carbon emission it is.

GetCarbonIntensity(ctx context.Context, region string) (float64, error)

I think it should change to this.

GetCarbonIntensity(ctx context.Context, region string) ([]CarbonIntensity, error)

The CarbonIntensity struct will have the data and can be converted to JSON for output by the CLI. I'm posting the JSON here as its easier to review.

[
    {
        "data_provider": "WattTime.org",
        "emissions_type": "marginal",
        "metric_type": "absolute",
        "region": "CAISO_NORTH",
        "units": "lb CO2 per MWh",
        "valid_from": "2022-08-01T08:20:00Z",
        "valid_to": "2022-08-01T08:25:00Z",
        "value": "939"
    },
    {
        "data_provider": "WattTime.org",
        "emissions_type": "marginal",
        "metric_type": "relative",
        "region": "CAISO_NORTH",
        "units": "percent",
        "valid_from": "2022-08-01T08:20:00Z",
        "valid_to": "2022-08-01T08:25:00Z",
        "value": "68"
    }
]

I think this covers the existing providers but this is just a first proposal for the JSON happy to improve on it.

@mrchrisadams I'd like to get your thoughts on this as well.

@mrchrisadams
Copy link
Member

hey Ross, it I understand it, you're proposing returning some kind of vector/list of value objects like the CarbonIntensity struct, right?

If it's idiomatic Go, I support the decision, and any kind of hint about what gets returned is good in my view.

I don't know how Go serialises objects like that to JSON - is it built into the standard library or do you need to write some serialiser?

C

@rossf7 rossf7 self-assigned this Aug 1, 2022
@rossf7
Copy link
Contributor

rossf7 commented Aug 1, 2022

Hi Chris,
yes, CarbonIntensity is a value object which in Go terminology is a struct. The idea is it will abstract the carbon intensity data into a format that can support any data provider.

The array is so we can handle WattTime which provides both relative and absolute values for marginal emissions. This also future proofs the design in case another provider also supports this.

The Go serialisation to JSON is built in. You just need to add some tags to the struct like here.

type GridIntensity struct {
	CountryCodeISO2              string  `json:"country_code_iso_2"`
}

@rossf7
Copy link
Contributor

rossf7 commented Aug 11, 2022

Changes are merged and will go in the next release.

@rossf7 rossf7 closed this as completed Aug 11, 2022
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

No branches or pull requests

3 participants