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

feature: Integrating MeteoBlue into the providers package #12

Open
2 tasks
yendelevium opened this issue Oct 13, 2024 · 15 comments
Open
2 tasks

feature: Integrating MeteoBlue into the providers package #12

yendelevium opened this issue Oct 13, 2024 · 15 comments
Labels
enhancement New feature or request NeedsDecision The issue is understood, but the best solution is undecided. Wait for a decision before writing code

Comments

@yendelevium
Copy link
Collaborator

yendelevium commented Oct 13, 2024

Feature Request

Summary

To integrate MB into the providers package

  • Structure MB models and processes
  • Unmarshal the data returned by MB into the baseData struct (plumber pkg)

Motivation

MB will be the primary provider for meteomunch, so proper integration is required. Then, the other data can be filled in by other providers like open-meteo, Copernicus etc

Detailed Design

The way baseData is structed, it has separate fields for current, hourly and daily data. MB's data is different, and it doesn't get cleanly translated into baseData.
We could create intermediate models for current, hourly and daily data, unmarshal MB data into all three separately, and then add them to the baseData struct.

Alternatives

  • Remove the current, hourly, and daily nested structs from baseData, and directly store the fields we want into the baseData struct. This removes the intermediate structs proposed in the first solution, at the cost of bloating up the baseData struct.

Additional Context

We would also need to pass this struct around to other providers(if available) to fill in data MB missed or doesn't provide, and then return this data back to the service which called it. The faster we can do this, the better.
We should also be able to extract the hourly,daily, and current data (incase we decide to go with the alternative option, I'm assuming we can probably access them via the baseData struct if we go with the proposed solution)

Acceptance Criteria

  • MB Data must be successfully translated to the baseData struct
  • The hourly, daily, and current data must be accessible separately

Additional Information

References #5

@yendelevium yendelevium added enhancement New feature or request NeedsDecision The issue is understood, but the best solution is undecided. Wait for a decision before writing code labels Oct 13, 2024
@yendelevium
Copy link
Collaborator Author

Any alternative solutions and/or outcomes are welcome. Please express your thoughts @adofm @raamsri

@raamsri
Copy link
Contributor

raamsri commented Oct 14, 2024

Good call Yash! Let's try to retain the idea of unmarshalling to BaseData struct until we run into roadblocks that break the design?

@raamsri
Copy link
Contributor

raamsri commented Oct 14, 2024

At a higher level, assuming the relevant new fields from MB are added to BaseStruct, I tried a two fold approach to work around the root level fields' name mismatch for seamless marshalling/unmarshalling.

I reckon that it's easier to show the approach via code for this one, so please see if this snippet makes sense.

package main

import (
	"encoding/json"
	"fmt"
)

type MyStruct struct {
	Field string `json:"field"`
}

func (m *MyStruct) UnmarshalJSON(data []byte) error {
	type Alias MyStruct
	aux := &struct {
		*Alias
		AltField string `json:"alt_field"`
	}{Alias: (*Alias)(m)}

	if err := json.Unmarshal(data, aux); err != nil {
		return err
	}

	if aux.AltField != "" {
		m.Field = aux.AltField
	}

	return nil
}

func main() {
	jsonData1 := `{"field": "value1"}`
	jsonData2 := `{"alt_field": "value2"}`

	var obj1, obj2 MyStruct

	if err := json.Unmarshal([]byte(jsonData1), &obj1); err != nil {
		fmt.Println("Error unmarshalling jsonData1:", err)
	}
	if err := json.Unmarshal([]byte(jsonData2), &obj2); err != nil {
		fmt.Println("Error unmarshalling jsonData2:", err)
	}

	fmt.Println("obj1:", obj1.Field) 
	fmt.Println("obj2:", obj2.Field) 
}

Output:
obj1: value1
obj2: value2

@raamsri
Copy link
Contributor

raamsri commented Oct 14, 2024

I have picked packages that are of most relevance from MB: basic-1h_basic-day_clouds-1h_clouds-day_air-1h_air-day_wind-1h_wind-day. So, this could be the APIPath for MB config.

QP would be apikey=xxxx&lat=10.9018379&lon=76.8998445&format=json&tz=GMT&forecast_days=1

The URI looks something like: https://my.meteoblue.com/packages/basic-1h_basic-day_clouds-1h_clouds-day_air-1h_air-day_wind-1h_wind-day?apikey=xxxx&lat=10.9018379&lon=76.8998445&format=json&tz=GMT&forecast_days=1

@adofm
Copy link
Collaborator

adofm commented Oct 14, 2024

we can start working on this @raamsri

@adofm
Copy link
Collaborator

adofm commented Oct 14, 2024

At a higher level, assuming the relevant new fields from MB are added to BaseStruct, I tried a two fold approach to work around the root level fields' name mismatch for seamless marshalling/unmarshalling.

I reckon that it's easier to show the approach via code for this one, so please see if this snippet makes sense.

package main

import (
	"encoding/json"
	"fmt"
)

type MyStruct struct {
	Field string `json:"field"`
}

func (m *MyStruct) UnmarshalJSON(data []byte) error {
	type Alias MyStruct
	aux := &struct {
		*Alias
		AltField string `json:"alt_field"`
	}{Alias: (*Alias)(m)}

	if err := json.Unmarshal(data, aux); err != nil {
		return err
	}

	if aux.AltField != "" {
		m.Field = aux.AltField
	}

	return nil
}

func main() {
	jsonData1 := `{"field": "value1"}`
	jsonData2 := `{"alt_field": "value2"}`

	var obj1, obj2 MyStruct

	if err := json.Unmarshal([]byte(jsonData1), &obj1); err != nil {
		fmt.Println("Error unmarshalling jsonData1:", err)
	}
	if err := json.Unmarshal([]byte(jsonData2), &obj2); err != nil {
		fmt.Println("Error unmarshalling jsonData2:", err)
	}

	fmt.Println("obj1:", obj1.Field) 
	fmt.Println("obj2:", obj2.Field) 
}

Output: obj1: value1 obj2: value2

this does make sense
like over here we are trying to handle different field names and mapping them

@yendelevium
Copy link
Collaborator Author

Yeah, looks good to me @raamsri . How did u think of this lol i could never (like actually i wanna know your thought proccess)

@raamsri
Copy link
Contributor

raamsri commented Oct 14, 2024

Nothing impressive but just a history with few years of sub-optimal design choices in the past 😅

Go's JSON package doesn't support multiple struct tags for the same field. And, I wince at the thought of maintaining new structs for every provider, so, I preferred a lazy solution as always. I've grudgingly applied similar logic for other projects where I had to transform(prefix, suffix, etc.) data before unmarshalling.

Though it may seem clean, I'm not quite content with the solution. At the worst case, extra JSON tags along with its mapping checks, grow linearly proportional to the total number of fields as new providers are added. So, I was honestly worried that you guys may not be happy with it. However, I do not have a better alternative.

@yendelevium
Copy link
Collaborator Author

@raamsri @adofm has any one started with this? If not i would like to take this up

@adofm
Copy link
Collaborator

adofm commented Oct 22, 2024

Yea I am working on this

@raamsri
Copy link
Contributor

raamsri commented Oct 22, 2024

It would be great to try out pair programming for this one, if you hadn't done it before. Do you both fancy it?

@raamsri
Copy link
Contributor

raamsri commented Oct 22, 2024

Nonetheless, @yendelevium there's more work on the plate. We haven't yet designed a way to orchestrate calls over the available meteo providers to fulfill the BaseData. There's a series of questions which we haven't sifted through yet. It may have to handle provider precedence as well when multi providers provide data for the same parameter, For instance, humidity is returned by both OM and MB, which one do we retain? In which order do we call? Does the order matter? When some of the fields couldn't be fulfilled for whatever reasons, should that be signaled separately for quick check without having to iterate the fields? If so then, what would be an efficient way to do that? And such things. It needs a new issue.

@adofm
Copy link
Collaborator

adofm commented Oct 22, 2024

It would be great to try out pair programming for this one, if you hadn't done it before. Do you both fancy it?

I am okay with it @raamsri @yendelevium

@yendelevium
Copy link
Collaborator Author

yendelevium commented Oct 22, 2024

It would be great to try out pair programming for this one, if you hadn't done it before. Do you both fancy it?

I am okay with it @raamsri @yendelevium

Same Here :D

@yendelevium
Copy link
Collaborator Author

Nonetheless, @yendelevium there's more work on the plate. We haven't yet designed a way to orchestrate calls over the available meteo providers to fulfill the BaseData. There's a series of questions which we haven't sifted through yet. It may have to handle provider precedence as well when multi providers provide data for the same parameter, For instance, humidity is returned by both OM and MB, which one do we retain? In which order do we call? Does the order matter? When some of the fields couldn't be fulfilled for whatever reasons, should that be signaled separately for quick check without having to iterate the fields? If so then, what would be an efficient way to do that? And such things. It needs a new issue.

IMO, we can have the providers we need only for some 'niche-r' fields be called first, and the 'main' providers later. This way, we can overwrite the data in a proper order ig? Since MB will be our primary provider(and I'm assuming the most trustworthy, but i have no grounds of proof) that will be called and all the data it gives will be retained.
I'm not so sure about how to signal fields which haven;t been filled w/o iterating through them.
But yes, a newer issue would definitely be needed. How about we create this after this issue is done?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request NeedsDecision The issue is understood, but the best solution is undecided. Wait for a decision before writing code
Projects
None yet
Development

No branches or pull requests

3 participants