Skip to content

Conversation

@albertoramosmonagas
Copy link
Contributor

What type of PR is this?

Add one of the following kinds:

  • repository management

What this PR does / why we need it:

Publication of Fall'25 M4 public release of predictive-connectivity-data v0.1.0

Which issue(s) this PR fixes:

Fixes #9

Special notes for reviewers:

None

Changelog input

 release-note
Publication of Fall'25 M4 public release of predictive-connectivity-data v0.1.0

Additional documentation

None

@github-actions
Copy link

github-actions bot commented Aug 19, 2025

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 2 0 0.02s
✅ REPOSITORY git_diff yes no 0.01s
✅ REPOSITORY secretlint yes no 0.58s
✅ YAML yamllint 1 0 0.35s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

Minor fixes in descriptions

Co-authored-by: Eric Murray <eric.murray@vodafone.com>
@albertoramosmonagas
Copy link
Contributor Author

Thanks for your review, @eric-murray! Very good points. I had missed those points, all of which have been resolved and applied to the PR.

@github-actions
Copy link

github-actions bot commented Aug 28, 2025

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 2 0 0.01s
✅ API spectral 1 0 1.67s
✅ GHERKIN gherkin-lint 1 0 0.32s
✅ REPOSITORY git_diff yes no 0.01s
✅ REPOSITORY secretlint yes no 0.61s
✅ YAML yamllint 1 0 0.51s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

eric-murray
eric-murray previously approved these changes Sep 3, 2025
Copy link
Contributor

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR - reviewed on the behalf of the release management team

  • (minor): I've added a NEW in the README.md as for the other APIs
  • (major): Subscription APIs must include 'notificationsBearerAuth' security scheme so probably it should be added in the yaml.

Thanks

@bigludo7 bigludo7 requested a review from a team September 10, 2025 07:52
@bigludo7
Copy link
Contributor

One additional request: please remove for the date-time attribute in the description Recommended format is yyyy-MM-dd'T'HH:mm:ss.SSSZ (i.e. which allows 2023-07-03T14:27:08.312+02:00 or 2023-07-03T12:27:08.312Z) as specified here
Thanks !

@albertoramosmonagas
Copy link
Contributor Author

Hi @bigludo7,

The other API Owner is on vacation, and we need approval to merge this PR.

@hdamker
Copy link
Contributor

hdamker commented Sep 11, 2025

@albertoramosmonagas @bigludo7

It seems that the API is using a simple one-time callback mechanism for the response and not subscription to events. With that it is not following the CAMARA Event Subscription & Notification Guide in multiple points.

I'm not sure if this is by intention and even then I would say that the API should use proper CloudEvent structure and a defined event type for the callback.

Or in other words:

Callback Notification Format: The API callback mechanism MUST use CloudEvents format according to CAMARA standards:

  • Current: application/json (line 255 in OAS file)
  • Required: application/cloudevents+json
  • The callback at line 254-257 must be updated to use CloudEvents format
  • The ConnectivityDataAsyncResponse schema (line 590) does not follow CloudEvents structure
  • CloudEvents requires specific attributes: id, source, specversion, type, datacontenttype, data, etc.
  • The actual connectivity data should be wrapped in the CloudEvents data field
  • This is a mandatory requirement per CAMARA Event Subscription & Notification Guide

With that I'm not comfortable to bypass the codeowner approvals and would also ask @bigludo7 to have a second look on the API definition.

@bigludo7 bigludo7 self-requested a review September 12, 2025 07:23
@bigludo7
Copy link
Contributor

Agree with @hdamker findings and sorry to have missed.

In particular the event structure must be aligned with our definition defined here and here.

As an example we should have this:

{
  "id": "123e4567-e89b-12d3-a456-426655440000",
  "source": "https://notificationSendServer12.supertelco.com",
  "type": "org.camaraproject.predictive-connectivity-datav0.connectivity-data-async",
  "specversion": "1.0",
  "time": "2023-01-17T13:18:23.682Z",
  "datacontenttype": "application/json",
  "data": {
    "layerThickness": 30,
    "timedConnectivityData": [
      {
      "startTime": "2024-01-03T11:00:00Z",
      "endTime": "2024-01-03T12:00:00Z",
      "cellConnectivityData": [
        {
          "geohash": "ezdmemd",
          "layerConnectivities": [
            "GC"
           ....
          ]
        },
        {
          "geohash": "ezdmemc",
          "layerConnectivities": [
            "GC",
           ....
          ]
        }
      ]
    },
    {
      "startTime": "2024-01-03T12:00:00Z",
      "endTime": "2024-01-03T13:00:00Z",
      "cellConnectivityData": [
        {
          "geohash": "ezdmemd",
          "layerConnectivities": [
            "GC",
            ...
          ]
        },
        {
          "geohash": "ezdmemc",
          "layerConnectivities": [
            "GC",
            ...
          ]
        }
      ]
    }
    ],
    "status": "SUPPORTED_AREA",
    "operationId": "2322f362-eaab-4cf3-86d2-efcbdf3a7cb4"
}

This modification should be probably done in a separate issue/PR.

@albertoramosmonagas
Copy link
Contributor Author

Hi @bigludo7 & @hdamker,

Thanks a lot for raising this point. From our side, we see it as follows:

  1. This should be aligned in the Predictive & Population WG, since both APIs share the same data model. In our view, the asynchronicity here is not so much about the evolution of the resource (i.e. event subscription), but rather about how the response is delivered. The synchronicity is therefore linked to the response model, not to the resource itself.
  2. As the Release Candidate has already been approved, we think it is better not to introduce functional changes at this stage. If there is consensus among all code owners (including Eric, who is currently on vacation), such adjustments could be considered for the next release cycle.
  3. In Population Density Data, the same asynchronous model has already been merged and approved, which provides consistency across related APIs.

To summarize: we implemented it this way originally because we thought it was the correct approach at that time. We are of course open to revisiting this after the current meta-release, and if the group agrees, we can apply changes in a future meta-release.

@hdamker
Copy link
Contributor

hdamker commented Sep 15, 2025

Hi @bigludo7 & @hdamker,

Thanks a lot for raising this point. From our side, we see it as follows:

  1. This should be aligned in the Predictive & Population WG, since both APIs share the same data model. In our view, the asynchronicity here is not so much about the evolution of the resource (i.e. event subscription), but rather about how the response is delivered. The synchronicity is therefore linked to the response model, not to the resource itself.

We have also other APIs which are using (implicit subscribed) event notifications to deliver there responses. We agreed with CAMARA Commonalities on one event structure across all APIs to avoid that API Consumers who are using multiple CAMARA APIS need to implement endpoints for different delivery models to receive asynchronous responses and notifications. To avoid a misunderstanding here: CloudEvents is only the delivery method, it does not restrict in any form the data model used within the payload.

A decision of a "Predictive & Population WG" to use a different delivery model is IMHO not compliant with the agreed Commonalities in CAMARA.

  1. As the Release Candidate has already been approved, we think it is better not to introduce functional changes at this stage. If there is consensus among all code owners (including Eric, who is currently on vacation), such adjustments could be considered for the next release cycle.

I agree that the change is a larger one and need to be discussed in the responsible team. But I want to be clear that the review of Release Management is not a design review but to ensure the needed release artifacts are there and consistent. Within the process the RM team has to rely on the statement of the API working group that the API definition and design is compliant with the need Commonalities release (here: r3.3, but the requirement to use CloudEvent existed already in r2.3). Currently it is by chance that certain non-compliant issues will be found, as in this case.

My proposed forward would be to create an issue for the needed change to get compliant with Commonalities r3.3 and then to update the API Readiness Checklist with a comment on the line regarding compliance with Commonalities. It then up to the Release Management, potentially TSC to discuss the consequences.

  1. In Population Density Data, the same asynchronous model has already been merged and approved, which provides consistency across related APIs.

Thanks for pointing to the fact that also Population Density Data has the same issue. That need to be corrected as well going forward to avoid fragmentation of the CAMARA APIs. That's sad as this API Repository is already in Incubating stage.

As said, it would be beyond the capabilities of the small team to do a deep design review for each of the 60 APIs. But we might need to introduce such a design review at latest before the incubation of an API Repository.

To summarize: we implemented it this way originally because we thought it was the correct approach at that time. We are of course open to revisiting this after the current meta-release, and if the group agrees, we can apply changes in a future meta-release.

I'm personally fine to include the initial version of PredictiveConnectivityData in the Fall25 meta-release even with the open issue (other initial v0.1.0 API version have for sure similar or other issues), but I wouldn't wait with the release of the next version until the next meta-release, but fix this issue as soon as possible and release a v0.2.0 (in accordance with the dual-phase model).

@bigludo7
Copy link
Contributor

bigludo7 commented Sep 15, 2025

Hello @albertoramosmonagas
I fully understand your point regarding the short delay & probably we should have spotted this before.

We have a consistency issue here in referring commonalities r3.3 but not following its definition.
I tend to think that the notification model is stable/mature as specified from cloudevents and all APIs must must be aligned.

Perhaps we can finish this release as it, but then quickly introduce a patch release (and not waiting Spring26) to fix this?
WDYT?

EDIT: I forget to publish my comment before lunch and published it back.... hopefully I'm pretty aligned with Herbert :)

@hdamker
Copy link
Contributor

hdamker commented Sep 15, 2025

EDIT: I forget to publish my comment before lunch and published it back.... hopefully I'm pretty aligned with Herbert :)

Yes, pretty aligned. Only point is that I think that this change justifies a 0.2.0 version, as it won't be backward compatible with the current definition.

@albertoramosmonagas
Copy link
Contributor Author

Hi @hdamker & @bigludo7,

Thanks a lot for the detailed feedback and clarifications.

  • We can create issues in both PredictiveConnectivityData and Population Density Data repositories to track CloudEvents compliance.
  • These could be addressed post-Fall25 in alignment with the WG.
  • The compliance fix might then be planned for v0.2.0, following the dual-phase model, so that both APIs stay consistently aligned across CAMARA.

In the meantime, our understanding is that we proceed with this PR and include the initial version in the Fall’25 meta-release. We can start moving these tasks in both repos right after Fall’25.

Would that approach work for you, or is there anything else you would expect us to capture or follow up on?

@bigludo7
Copy link
Contributor

One thing @albertoramosmonagas ... as suggested by @hdamker you can update the API Readiness Checklist with a comment on the line regarding compliance with Commonalities.
Then happy to review

@hdamker Once we have the comment, do I approve or do we have to wait TSC?

@albertoramosmonagas
Copy link
Contributor Author

@hdamker @bigludo7,

Comment created in the API Readiness Checklist and issue created for Predictive Connectivity Data (#38) and for Population Density Data (camaraproject/PopulationDensityData#105)

@bigludo7
Copy link
Contributor

@albertoramosmonagas Perhaps you can improve the formatting as it adds 2 lines - probably not what you're expecting.

You can add an asterisk & put a note after the table.

@albertoramosmonagas
Copy link
Contributor Author

@albertoramosmonagas Perhaps you can improve the formatting as it adds 2 lines - probably not what you're expecting.

You can add an asterisk & put a note after the table.

I think it's done on my side

@bigludo7
Copy link
Contributor

Look good for me !
Up now to check with @hdamker if we can move forward with M4 today and have a sync during next TSC.

@hdamker
Copy link
Contributor

hdamker commented Sep 16, 2025

Look good for me !
Up now to check with @hdamker if we can move forward with M4 today and have a sync during next TSC.

@albertoramosmonagas @bigludo7 Looks good for me too, a good and pragmatic solution. From my perspective we can go forward.

@albertoramosmonagas
Copy link
Contributor Author

@hdamker @bigludo7
Thanks for the review. We would then only need two approvals to merge it.

@hdamker
Copy link
Contributor

hdamker commented Sep 16, 2025

Thanks for the review. We would then only need two approvals to merge it.

Technically you need two approvals: one from release management reviewer (that will come from @bigludo7 who runs the review) and one from a code owner. If @eric-murray is not available, I can help to bypass the rule later.

@albertoramosmonagas
Copy link
Contributor Author

Thanks for the review. We would then only need two approvals to merge it.

Technically you need two approvals: one from release management reviewer (that will come from @bigludo7 who runs the review) and one from a code owner. If @eric-murray is not available, I can help to bypass the rule later.

I understand @eric-murray is on holidays for most of the month and may only be checking in occasionally to support some of the releases he owns. From what we discussed, it sounded like he had already given his approval prior to the recent format updates, so everything seemed aligned on his side.

From our end, we truly appreciate your support and don’t foresee any issues in moving forward with your and @bigludo7 approval.

Copy link
Contributor

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @albertoramosmonagas

Approved on behalf of Release Management 👏
Next steps for the team:
• [ ] PR merged (by API repository codeowner)
• [ ] Release created within GitHub (by API repository codeowner)
• [ ] Release Tracker updated (with creation date of the release and the release tag link)

@albertoramosmonagas
Copy link
Contributor Author

@hdamker

I thought that with your approvals I would be able to merge, but I'm still stuck with this: Merging is blocked due to falling in merge requirements (Awaiting code owner review by eric-murray).

@hdamker
Copy link
Contributor

hdamker commented Sep 16, 2025

I understand @eric-murray is on holidays for most of the month and may only be checking in occasionally to support some of the releases he owns. From what we discussed, it sounded like he had already given his approval prior to the recent format updates, so everything seemed aligned on his side.

@albertoramosmonagas
Yes, I see @eric-murray's previous approval and that there are no critical updates since then. With that I will merge the PR now, bypassing the rule of a needed code owner approval. Please pick up from there and create the release.

@hdamker hdamker merged commit e4cc832 into main Sep 16, 2025
2 checks passed
@hdamker hdamker deleted the v0.1.0-public branch September 16, 2025 14:03
@albertoramosmonagas
Copy link
Contributor Author

@bigludo7
Next steps for the team:

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

Successfully merging this pull request may close these issues.

Scope for Fall25 releases (in preparation)

5 participants