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

feat: API Sync by GitHub Action for @ctreatma #10

Merged
merged 3 commits into from
Dec 15, 2023
Merged

Conversation

github-actions[bot]
Copy link
Contributor

This API Sync PR was triggered by @ctreatma through GitHub Actions workflow_displatch
on 2023-12-13.

  • latest Swagger is fetched
  • patches have been applied
  • generated client has been updated

Port *Href `json:"port,omitempty"`
Project *Href `json:"project,omitempty"`
Bill *bool `json:"bill,omitempty"`
BillType NullableVlanVirtualCircuitBillType `json:"bill_type,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Interesting type introduced here: NullableVlanVirtualCircuitBillType

Copy link
Member

Choose a reason for hiding this comment

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

Curious if Nullable is a necessary distinction or if the spec for the attribute is more verbose in definition than others and this is what that looks like.

Without digging into this, I would have expected:

BillType *VlanVirtualCircuitBillType

Copy link
Member

@displague displague Dec 13, 2023

Choose a reason for hiding this comment

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

@displague
Copy link
Member

displague commented Dec 15, 2023

non directly related, but this PR suggests that we'll need a way to generate sync PRs for each and every spec (perhaps all in one job)

@ctreatma
Copy link
Contributor

ctreatma commented Dec 15, 2023

The service-specific sync job is created automatically when the service is onboarded. The current job structure is based on the opinion that we should expressly not sync all specs in one PR, because one PR that changes multiple services will require massive coordination just to review it. On top of that, it's not entirely clear how broadly applicable a GitHub Actions job for spec syncing is. Metal needs it because the spec URL is unversioned, and it looks like LBaaS will need it for the same reason, at least initially. Fabric is pulled from SwaggerHub using a versioned URL, so it's debatable whether a sync job is actually useful for that.

More generally, it's possible the best outcome is for the sync job to go away completely. In my opinion the ideal process is for the SDK to be updated as part of the API development process rather than after production deployment; then teams can use a branch build of the SDK to validate that their changes work the way customers need them to work.

@ctreatma ctreatma merged commit 8096b49 into main Dec 15, 2023
@ctreatma ctreatma deleted the sync/gh-1702481729 branch December 15, 2023 17:23
Copy link
Contributor Author

This PR is included in version 0.31.0 🎉

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.

2 participants