tinkershack / meteomunch

Munch is a package/webserver that assimilates select set of meteo data from multiple weather providers.
MIT License
2 stars 3 forks source link

feature: Integrating MeteoBlue into the providers package #12

Open yendelevium opened 1 month ago

yendelevium commented 1 month ago

Feature Request

Summary

To integrate MB into the providers package

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

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

Additional Information

References #5

yendelevium commented 1 month ago

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

raamsri commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

we can start working on this @raamsri

adofm commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

adofm commented 1 month ago

Yea I am working on this

raamsri commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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?