-
Notifications
You must be signed in to change notification settings - Fork 3
Fix breaking APIs #45
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
Conversation
WalkthroughThis PR renames organization user timestamps from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/provider/data_source_openai_project.go (1)
101-104: Uselog.Printfinstead offmt.Printffor consistency.These debug statements use
fmt.Printfwhich writes to stdout, while the rest of the codebase (including this file's other logging at line 100+) useslog.Printf. This inconsistency can cause issues with log aggregation and filtering.- fmt.Printf("Getting project with ID: %s\n", projectID) - fmt.Printf("Using URL: %s\n", strings.Replace(url, client.APIURL, "", 1)) - fmt.Printf("OpenAI client config: API URL=%s, Organization ID=%s\n", client.APIURL, client.OrganizationID) - fmt.Printf("Making API request: GET %s\n", url) + log.Printf("[DEBUG] Getting project with ID: %s", projectID) + log.Printf("[DEBUG] Using URL: %s", strings.Replace(url, client.APIURL, "", 1)) + log.Printf("[DEBUG] OpenAI client config: API URL=%s, Organization ID=%s", client.APIURL, client.OrganizationID) + log.Printf("[DEBUG] Making API request: GET %s", url)You'll also need to add
"log"to the imports.internal/provider/resource_openai_project.go (1)
17-21: Panic recovery may hide bugs silently.This defer/recover pattern silently logs panics as warnings and continues execution. While this prevents crashes, it can mask serious bugs and leave resources in inconsistent states. Consider whether this is truly necessary or if panics should propagate.
If panic recovery is genuinely needed, consider at least returning an error to the caller:
func resourceOpenAIProject() *schema.Resource { - defer func() { - if r := recover(); r != nil { - log.Printf("[WARN] Recovered from panic in resourceOpenAIProject: %v", r) - } - }() + // Note: Removed panic recovery - panics should propagate to reveal bugs
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/data-sources/organization_user.md(1 hunks)docs/data-sources/organization_users.md(1 hunks)docs/data-sources/project.md(3 hunks)docs/data-sources/projects.md(2 hunks)docs/resources/organization_user.md(1 hunks)docs/resources/project.md(3 hunks)internal/client/client.go(8 hunks)internal/provider/data_source_openai_organization_user.go(2 hunks)internal/provider/data_source_openai_organization_users.go(3 hunks)internal/provider/data_source_openai_project.go(3 hunks)internal/provider/data_source_openai_projects.go(2 hunks)internal/provider/resource_openai_organization_user.go(4 hunks)internal/provider/resource_openai_project.go(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
internal/provider/data_source_openai_organization_user.go (1)
internal/provider/data_source_openai_project_user.go (2)
dataSourceOpenAIProjectUserRead(139-218)dataSourceOpenAIProjectUser(16-49)
internal/provider/data_source_openai_organization_users.go (1)
internal/provider/data_source_openai_project_users.go (1)
dataSourceOpenAIProjectUsers(14-83)
internal/client/client.go (3)
internal/provider/provider.go (1)
OpenAIClient(27-31)internal/provider/resource_openai_fine_tuning_job.go (1)
Project(47-51)internal/provider/resource_openai_project_user.go (3)
resourceOpenAIProjectUser(18-58)resourceOpenAIProjectUserUpdate(259-348)interface{}(395-395)
internal/provider/resource_openai_project.go (3)
internal/provider/resource_openai_upload.go (1)
resourceOpenAIUpload(43-103)internal/provider/resource_openai_project_user.go (3)
resourceOpenAIProjectUser(18-58)resourceOpenAIProjectUserUpdate(259-348)resourceOpenAIProjectUserImport(395-435)internal/provider/resource_openai_invite.go (1)
resourceOpenAIInvite(13-79)
🪛 GitHub Actions: CI
docs/resources/project.md
[error] 1-1: Process completed with exit code 1. CI step failed after diffs in docs/resources/project.md.
🔇 Additional comments (32)
internal/provider/data_source_openai_projects.go (4)
48-52: Breaking change: field renamed fromnametotitle.This reflects the OpenAI API change where projects now use
titleinstead ofname. Users will need to update their Terraform configurations to referencetitleinstead ofnamewhen this change is deployed.
58-62: LGTM: field renamed fromcreated_attocreated.This change aligns with the new OpenAI API response format.
63-72: LGTM: new fields added.The
archived_atandis_initialfields are new additions that enhance project management capabilities. These are additive (non-breaking) changes.
207-216: LGTM: mappings updated correctly.The field mappings now use
project.Title,project.Created,project.IsInitial, and conditionally setarchived_atwhen non-nil. The nil check forArchivedAtis appropriate since it's an optional pointer field.internal/client/client.go (6)
143-151: Major Project struct restructuring to match new API format.The Project struct has been significantly simplified to match the new OpenAI API response. Removed fields include
Name,Description,CreatedAt, and several nested objects. The new structure usesTitleinstead ofNameandCreatedinstead ofCreatedAt, with new fieldsArchivedAt(as a nullable pointer) andIsInitial.
194-195: LGTM: CreateProjectRequest updated to usetitle.The request struct now uses
titleinstead ofname, matching the new API contract.
562-611: LGTM: new user data structures with conversion helper.The introduction of
UserInfo,OrganizationUser, andUsertypes with aToUser()conversion method is a clean approach to handling the new nested API response structure. The conversion method correctly flattens user information fromou.Userfields (ID, Email, Name) while preserving organization-level fields (Role, Created, IsDefault, etc.).
615-620: LGTM: UsersResponse updated to use OrganizationUser array.The
Datafield now uses[]OrganizationUserto match the new API response structure. Consumers of this response should use theToUser()method for easier field access.
681-765: LGTM: user API methods updated to handle new data structure.
FindUserByEmail,GetUser, andUpdateUserRoleall correctly unmarshal the API response asOrganizationUserand convert to the flattenedUserstruct viaToUser(). This maintains a consistent return type while adapting to the new API format.
1114-1196: LGTM: project creation and update methods usetitle.Both
CreateProjectandUpdateProjectmethods have been updated to usetitleas the parameter name and in the request body. Debug messages also reference "title" consistently.docs/data-sources/organization_user.md (1)
25-25: LGTM: documentation updated to reflect field rename.The documentation correctly reflects the rename from
added_attocreatedfor the organization user data source.docs/data-sources/organization_users.md (1)
49-49: LGTM: documentation updated for nested schema.The nested users schema documentation correctly shows
createdinstead ofadded_at.docs/resources/organization_user.md (1)
44-44: LGTM: resource documentation updated.The resource documentation correctly reflects the
createdfield rename.internal/provider/data_source_openai_organization_user.go (2)
41-45: LGTM: schema field renamed fromadded_attocreated.The data source schema correctly uses
createdto match the new API response format.
117-119: LGTM: field mapping updated.The code correctly sets
createdfromuser.Createdwith an appropriate error message.internal/provider/resource_openai_organization_user.go (2)
49-53: LGTM: resource schema field renamed.The resource schema correctly uses
createdinstead ofadded_at.
101-103: LGTM: all CRUD operations updated consistently.All resource CRUD operations (Create, Read, Update) correctly set the
createdfield fromuser.Createdwith appropriate error messages.Also applies to: 144-146, 176-178
internal/provider/data_source_openai_organization_users.go (3)
61-65: LGTM: schema field renamed tocreated.The users list schema correctly uses
createdfor the timestamp field.
103-109: LGTM: single user mapping updated.When fetching a specific user by ID, the mapping correctly uses
user.Createdfor thecreatedfield.
147-159: LGTM: paginated user list updated with conversion helper.The pagination loop correctly:
- Iterates over
resp.Datawhich is now[]OrganizationUser- Renames the loop variable to
orgUserfor clarity- Converts each
orgUserto a flattenedUserviaToUser()- Maps all fields including
user.Createdto thecreatedfieldThis ensures consistent output structure across both single-user and paginated cases.
docs/resources/project.md (3)
15-31: Example usage correctly updated to usetitlefield.The example usage properly demonstrates the new
titlefield and removes the deprecateddescriptionfield. This aligns with the API schema changes.
36-50: Schema documentation reflects the new API structure.The schema correctly documents:
titleas requiredarchived_atandcreatedas Number (timestamps)is_initialas BooleanThis matches the changes in
resource_openai_project.go.
5-6: The description block in lines 5-6 is intentionally empty and is consistent with all other auto-generated resource documentation files in this repository. Since this file is auto-generated by terraform-plugin-docs (as noted in the header), the empty description format is expected and not a cause for concern. The pipeline failure is unrelated to the empty description.Likely an incorrect or invalid review comment.
docs/data-sources/projects.md (1)
40-50: Nested schema forprojectscorrectly documents new field structure.The nested schema accurately reflects the updated API response structure with
archived_at,created,is_initial, andtitlefields replacing the oldcreated_atandnamefields.internal/provider/data_source_openai_project.go (2)
16-24: ProjectResponse struct correctly reflects the new API schema.The struct properly maps the updated API fields with appropriate types:
Createdasint64for Unix timestampArchivedAtas*int64to handle nullable timestampsIsInitialasbool
158-165: Nil check forArchivedAtis handled correctly.The code properly checks for nil before dereferencing the pointer, preventing potential nil pointer dereferences. The
is_initialfield is correctly set without needing a nil check since it's a bool with a zero value default.internal/provider/resource_openai_project.go (4)
36-61: Schema correctly reflects the updated API structure.The schema properly defines:
titleas required (replacingname)createdasTypeInt(replacingcreated_atstring)archived_atasTypeIntis_initialasTypeBool(new field)This is consistent with the data source schema and documentation.
125-134: Consistent handling of nullableArchivedAtfield.The nil check before dereferencing
project.ArchivedAtis correct and consistent with the data source implementation. Theis_initialfield is properly set.
75-83: VerifyCreateProjectclient method signature.The call to
c.CreateProject(title)assumes the client method accepts only a title parameter. Ensure the client implementation in the OpenAI SDK matches this signature and confirm whether the parameters have changed from a previous version that may have accepted name, description, and isDefault.
148-156: TheUpdateProjectmethod signature is correct.The client method is defined as
func (c *OpenAIClient) UpdateProject(id, title string) (*Project, error)in internal/client/client.go, which matches the callc.UpdateProject(d.Id(), title)exactly—both id and title are passed as strings, and the error return value is properly captured.docs/data-sources/project.md (2)
26-32: Example correctly demonstrates the updatedtitlefield.The locals block properly shows the renamed field access from
nametotitle, helping users migrate their configurations.
46-53: Read-Only schema documentation is accurate.The schema correctly documents the new fields (
archived_at,created,is_initial,title) with appropriate types, matching the implementation indata_source_openai_project.go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/resources/project.md (1)
15-31: Example usage should reflect all required and new fields.The example shows how to create projects with
title, but users may benefit from seeing a more complete example that includes optional fields and demonstrates accessing read-only fields. Consider expanding the example to show how to reference the new fields in outputs (e.g.,is_initial,created).Expand the example to be more instructive:
# Create a new OpenAI project resource "openai_project" "development" { title = "Development Project" } # Create a production project resource "openai_project" "production" { title = "Production API Services" } # Output the project ID output "dev_project_id" { value = openai_project.development.id description = "The ID of the development project" } + +# Access read-only fields +output "dev_project_created" { + value = openai_project.development.created + description = "Unix timestamp when the development project was created" +} + +output "dev_is_initial" { + value = openai_project.development.is_initial + description = "Whether this is the initial project" +}examples/resources/openai_project/resource.tf (1)
2-9: Example usage is consistent with documented schema changes.The resources correctly use the new
titleattribute. However, users accustomed to thenameanddescriptionattributes should be warned of this breaking change in the example comments.Add a clarifying comment to help users understand the changes:
# Create a new OpenAI project +# Note: The 'title' field replaces the previous 'name' and 'description' fields resource "openai_project" "development" { title = "Development Project" } # Create a production project resource "openai_project" "production" { title = "Production API Services" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/data-sources/project.md(2 hunks)docs/data-sources/projects.md(1 hunks)docs/resources/project.md(2 hunks)examples/data-sources/openai_project/data-source.tf(1 hunks)examples/resources/openai_project/resource.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/data-sources/projects.md
- docs/data-sources/project.md
🔇 Additional comments (1)
examples/data-sources/openai_project/data-source.tf (1)
16-16: Example usage correctly updated to reflect schema changes.The local variable rename from
project_nametoproject_titleand the attribute access change from.nameto.titleare consistent with the documented schema changes. However, note that this is a breaking change for users with existing Terraform configurations referencing the.nameattribute.Verify that migration guidance or deprecation warnings are provided to users upgrading to this version, as the removal of the
.nameattribute will break existing configurations.
| - `title` (String) The title of the project | ||
|
|
||
| ### Optional | ||
|
|
||
| - `description` (String) A description of the project | ||
| - `timeouts` (Block, Optional) (see [below for nested schema](#nestedblock--timeouts)) | ||
|
|
||
| ### Read-Only | ||
|
|
||
| - `archived_at` (String) Timestamp when the project was archived, if applicable | ||
| - `created_at` (String) Timestamp when the project was created | ||
| - `archived_at` (Number) Timestamp when the project was archived, if applicable | ||
| - `created` (Number) Timestamp when the project was created | ||
| - `id` (String) The ID of this resource. | ||
| - `is_initial` (Boolean) Whether this is the initial project | ||
| - `status` (String) Status of the project (active, archived, etc.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaking schema changes lack migration guidance.
The documentation reflects significant breaking changes but does not provide migration guidance:
- Removed fields (no longer accessible):
description,usage_limits— users relying on these will encounter errors - Type changes (String → Number):
archived_atandcreated— users with string parsing logic will break - Field rename (created_at → created): Affects both references and potentially parsing expectations
Users upgrading to this version will experience silent failures or errors if their configurations or parsing logic depend on the old field names or types.
Add a Breaking Changes section to the documentation to inform users:
### Breaking Changes
- **`name` → `title`**: The project name attribute has been renamed to `title`
- **`created_at` → `created`** with type change (String → Number): Now returns a Unix timestamp
- **`archived_at`** type change (String → Number): Now returns a Unix timestamp
- **Removed fields**: `description` and `usage_limits` are no longer available
- **New field**: `is_initial` (Boolean) indicates if the project is the initial/default project🤖 Prompt for AI Agents
In docs/resources/project.md around lines 38-50, add a "Breaking Changes"
section that enumerates field renames, type changes, and removals and provides
brief migration guidance: list name→title, created_at→created (String→Number)
and archived_at (String→Number) with instruction to convert/expect Unix
timestamps instead of parsing strings, note removed fields description and
usage_limits and recommend alternatives or fallbacks, and call out the new
is_initial Boolean; ensure the section clearly maps old field names/types to new
ones and gives one-line steps developers can take to update parsing/validation
and migrate stored data.
It seems OpenAI broke at least 2 administrative APIs recently. This changes are not documented on https://platform.openai.com/docs/api-reference/, but endpoints return data in a different format.
Summary by CodeRabbit
added_at→created,name→title, andcreated_at→created.archived_atandis_initial.descriptionand theusage_limitsnested schema.title/createdconsistently.✏️ Tip: You can customize this high-level summary in your review settings.