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

[management] Project development roadmap #657

Open
jaimeventura opened this issue Jul 21, 2021 · 6 comments
Open

[management] Project development roadmap #657

jaimeventura opened this issue Jul 21, 2021 · 6 comments

Comments

@jaimeventura
Copy link

Hey,

Context
Im investigating an issue related to missing parameter. Im using the version on the development branch (up to commit 8a6d611):

curl 127.0.0.1:8099/agency/vehicles/8869471e-0ce8-42b1-9cd6-de043260f19a/event  -H "Content-Type: application/json" -H 'Authorization: Bearer eyJhbxxxxxxxxxxxxcXXxxxxxJqI' -XPOST -d '{"event_type":"trip_start","timestamp":1626795564136,"telemetry":{"device_id":"8869471e-0ce8-42b1-9cd6-de043260f19a","timestamp":1626795564136,"gps":{"lat":41.143001014744,"lng":-8.61387095508788},"charge":0.94},"trip_id":"887ee4ed-17ca-44a3-9047-7cd4ba5083f2"}'
{"error":"missing_param","error_description":"missing enum field \"event_type\""}

As you can see, im sending the event_type in the body, but mds-agency complains about it.

After pulling to the latest commit (8e1dd6c), i get a different error:

{"error":"missing_param","error_description":"A required parameter is missing.","error_details":["event_types"]}

( the request is the same, im just posting the reply to save space in the post)

Note that, according to the message, the request is now missing event_types (plural).
At first i thought this was a typo, but looking at the code, that doesnt seem to be the case. My feeling is that its now using a new mds spec.

Looking at the latest(?) specification, there's no such attribute.

Questions
Let me first acknowledge that using a dev branch is risky and prone to hit in these kind of situations.

But, is there a roadmap of features do be developed i could see?

Can i see which mds spec version does a mds-core version/branch complies with?

I see that there's a branch release/0.1.1(although the latest release states version "0.1.0") is this the latest 'stable' version?

Thank you

@avatarneil
Copy link

Hi @jaimeventura,

To first address the particular issue you're running into; it seems like you're attempting to send event_type in the payload (which would be what you'd send for MDS 0.4.1 and earlier), but MDS 1.0 and forward expects event_types note the singular vs plural difference.

MDS 0.4.1 is quite out of date, and we're working on updating our tenants to MDS 1.0.0+ which is why the develop branch contains code for that. release/0.1.1 is the last branch which supports MDS 0.4.1, however we're currently maintaining a support/mds-0.4.1 branch for one of our clients that has some additional commits to it, so you can try using either branch and seeing which works best for you.

One thing that's rather challenging is that we sometimes need to add features/extend MDS in ways that are not currently ratified in OMF versions of the specification; for example, mds-core now has support for multi-modality, which is not currently in the OMF MDS standard. We are attempting to maintain compatibility with major MDS versions, while potentially having some extra bells and whistles available for use too. On that note, at the moment the develop branch of MDS-Core should be largely 1.0.0 compliant, however there may be some places where extra properties leak out (we have yet to mark this 1.0.0 compatible version of the codebase as a QA'd 1.0.0 release, but that should be happening within the next month or so); that being said, any payloads that are 1.0 compliant should persist without any troubles whatsoever.

Unfortunately, we don't have a public feature roadmap for mds-core at the moment, but I can talk with the team and see if there's an appetite for someone to take on maintaining that.

I don't think we currently track in a README what version of MDS a given branch is compatible with, but we should probably start doing that; would you mind making an issue specifically to start tracking supported major versions in the README?

@jaimeventura
Copy link
Author

Hey,
Thanks for the quick reply.
So, about the issue, you explanation is pretty much what i thought initially. Note that, by explaining that issue, i was just trying detail the context of my doubt (the reason for my question). If you had some kind of roadmap, i would just consult it and understand that you were implementing the change.
Any way, thank you for you explanation.

Ill draw my attentions to either release/0.1.1 or support/mds-0.4.1 'cause that's what the providers i work with are supporting.

Regarding opening an issue, i'll do it right away.

Allow me to give you an idea. It would be great of you could support several versions of the API/spec (for instance using "/v0.4/vehicles/sfkjshddfgjkljkh/event" or sticking the version to a header,...). I understand that you still working towards a 1.0 version and this would be the least of your concerns and, possibly, even impossible to have the same app supporting multiple version. But it would be really great as i have this feeling MDS is gaining traction around the world (not just the US), and this feature would definitely help to migrate from previous versions to 1.0.

Thanks,

@avatarneil
Copy link

@jaimeventura 100% agree about supporting multiple versions on the same instance, this is something we hope to be able to do across minor version bumps (e.g. 1.1, 1.2, etc...). Our current plan for migrating 0.4.1 tenants -> 1.0.0 is to run parallel instances, and convert payloads from 0.4.1 -> 1.0.0 using a transformer on a Kafka stream; this ensures that worst-case-scenario if a transform fails or is lossy, we have a complete 0.4.1 db still. @drtyh2o might be able to add some more details as he's owning that work!

@jaimeventura
Copy link
Author

Hey,
Our e-scooter service provider moved to supporting version 1.0.0 of MDS and we found a new issue.
We're using the code in 'master' branch.
Issuing the following requeste results in a 400 reply:

curl https://mds.services.dev.portodigital.pt/agency/vehicles/a7990baa-909e-42ed-8546-a2a9d2e42b3c/event \
-H "Content-Type: application/json" \
-H 'Authorization: Bearer eyJhbGcic…35Dkdzi2dJqI' \
  --data '
{
  "device_id": "a7990baa-909e-42ed-8546-a2a9d2e42b3c",
  "vehicle_state": "elsewhere",
  "event_types": [
    "trip_leave_jurisdiction"
  ],
  "telemetry": {
    "device_id": "a7990baa-909e-42ed-8546-a2a9d2e42b3c",
    "gps": {
      "lat": 41.14726333333333,
      "lng": -8.674806666666665,
      "accuracy": 0
    },
    "charge": 0.45,
    "timestamp": 1634734079000
  },
  "trip_id": "6e073f7f-b955-4033-b7bd-6237faa51507",
  "timestamp": 1634734083000
}
'

The result is:

{"error":"missing_param","error_description":"A required parameter is missing.","error_details":["provider_id"]}

According to spec for 1.0.0, no "provider_id" is required for "events",
In this comment you mention that you "sometimes need to add features/extend MDS in ways that are not currently ratified in OMF versions of the specification; for example, mds-core now has support for multi-modality".

Is what im presenting related to that?

Is there a mds-core version/branch fully compliant version 1.0.0, i mean without extra features that are not in MDS specification? or a way to disable these extra features?

Thanks

@avatarneil
Copy link

@jaimeventura This is a bug (unrelated to extra features) that was fixed in #754 (which should be in the master branch now). Can you confirm what commit of master you're running, and if the latest version resolves this issue?

As far as disabling the extra features, this is not something that is currently fully supported throughout all of the APIs, but we're working on it :) The plan is that APIs will look for some extra component in the version header to differentiate, but this likely won't ship until end of Q4 at earliest. As far as agency intake goes, there should be no spec+ properties that are required. That being said, we have identified that there are some properties in response bodies that are not in the spec, and we're working on filtering those out as we speak (for example, #797).

@jaimeventura
Copy link
Author

Thank you, it works!

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

2 participants