Skip to content

feat(mexp): Onboarding Modelexperiments - #1547

Open
paul-sffrth wants to merge 45 commits into
stackitcloud:mainfrom
paul-sffrth:feat/mlflow
Open

feat(mexp): Onboarding Modelexperiments#1547
paul-sffrth wants to merge 45 commits into
stackitcloud:mainfrom
paul-sffrth:feat/mlflow

Conversation

@paul-sffrth

Copy link
Copy Markdown
Member

Description

This is the onboarding PR for integration STACKIT Modelexperiments into the STACKIT Terraform Provider.

It adds for customer the ability to:

  • Create, Read, Update and Delete Modelexperiments Instances
  • Create, Read, Update and Delete Mdelexperiments Instance Tokens

Checklist

  • Issue was linked above
  • Code format was applied: make fmt
  • Examples were added / adjusted (see examples/ directory)
  • Docs are up-to-date: make generate-docs (will be checked by CI)
  • Unit tests got implemented or updated
  • Acceptance tests got implemented or updated (see e.g. here)
  • Unit tests are passing: make test (will be checked by CI)
  • No linter issues: make lint (will be checked by CI)

@paul-sffrth
paul-sffrth requested a review from a team as a code owner June 29, 2026 09:24
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it.

@github-actions github-actions Bot added the Stale PR is marked as stale due to inactivity. label Jul 8, 2026
@Fyusel Fyusel removed the Stale PR is marked as stale due to inactivity. label Jul 8, 2026
Comment thread examples/resources/stackit_modelexperiments_token/resource.tf
Comment thread stackit/internal/services/modelexperiments/instance/description.md Outdated
Comment thread stackit/internal/services/modelexperiments/instance/resource.go Outdated
Comment thread stackit/internal/services/modelexperiments/instance/resource.go
Comment thread stackit/internal/services/modelexperiments/token/description.md Outdated
Comment thread stackit/internal/services/modelexperiments/utils/util.go Outdated
Comment thread stackit/internal/services/modelexperiments/token/resource.go Outdated
Comment thread stackit/internal/services/modelexperiments/instance/resource.go Outdated
Comment thread stackit/internal/services/modelexperiments/token/resource.go Outdated
Comment thread stackit/internal/services/modelexperiments/token/resource.go
Comment thread stackit/internal/services/modelexperiments/token/resource.go Outdated
Comment thread stackit/internal/services/modelexperiments/token/resource.go Outdated
Comment thread stackit/internal/services/modelexperiments/token/resource.go
Comment thread stackit/internal/services/modelexperiments/instance/resource.go Outdated
Comment thread stackit/internal/services/modelexperiments/instance/resource.go Outdated
Comment thread stackit/internal/services/modelexperiments/token/resource.go Outdated
Comment thread stackit/internal/services/modelexperiments/modelexperiments_acc_test.go Outdated
@Fyusel

Fyusel commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

After those comments are resolved I will start to run the acceptance tests and manual testing

Comment thread stackit/internal/services/modelexperiments/instance/datasource.go
Comment thread stackit/internal/services/modelexperiments/token/datasource.go
Comment thread examples/resources/stackit_modelexperiments_instance/resource.tf
Comment thread examples/resources/stackit_modelexperiments_token/resource.tf Outdated
Comment thread examples/resources/stackit_modelexperiments_token/resource.tf Outdated
Fyusel
Fyusel previously approved these changes Jul 24, 2026
Comment thread examples/resources/stackit_modelexperiments_token/resource.tf Outdated
Comment thread stackit/internal/services/modelexperiments/instance/resource.go Outdated
Comment on lines +498 to +500
if instance.Id == "" {
return fmt.Errorf("instance id not present")
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

check if instance == nil is missing. Potential nil pointer exception

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

check is still missing

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

something like this

Suggested change
if instance.Id == "" {
return fmt.Errorf("instance id not present")
}
if instance == nil || instance.Id == "" {
return fmt.Errorf("instance or instance id not present")
}

Comment thread stackit/internal/services/modelexperiments/instance/datasource.go Outdated
Comment thread stackit/internal/services/modelexperiments/token/datasource.go Outdated
return fmt.Errorf("model input is nil")
}

if token.Id == "" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nil check for token is missing

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here, nil check is still missing

Comment thread stackit/internal/services/modelexperiments/token/resource.go Outdated
return fmt.Errorf("model input is nil")
}

if token.Id == "" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

potential nil pointer on token

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

still not fixed

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it.

@github-actions github-actions Bot added the Stale PR is marked as stale due to inactivity. label Aug 3, 2026
@Fyusel Fyusel added ignore-stale and removed Stale PR is marked as stale due to inactivity. labels Aug 4, 2026
Fyusel
Fyusel previously approved these changes Aug 5, 2026
@marceljk

marceljk commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@paul-sffrth I reopened some of my comments from the previous review because they are still not fixed. The nil check should be part of the mapFields function. If you have any questions, feel free to reach out

@paul-sffrth

Copy link
Copy Markdown
Member Author

I think checking on nil in the map functions don't make sense in that case, since the Instance or Token struct cannot be nil in the response struct returned by the SDK (e.g. the CreateInstanceResponse struct). The API request returns a pointer to the response struct and if the response struct is not nil then the Instance or Token object that gets mapped in the map function cannot be nil. If the response struct is nil, then the nil pointer dereference would already happen when calling the map function (for example here). Therefore I decided to check for nil before calling the map function (for example here)

@marceljk

marceljk commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Checking if the passed pointer attributes are not nil should be checked in my opinion every time, independent of whether it's checked before calling the function. Someone else could use the function in the future somewhere else, and then it's possible to get a panic because there it's possible to pass a nil pointer.

In our contribution guide we have also nil checks in the mapFields function, so it's the way to go in our provider:

func mapFields(barResp *foo.GetBarResponse, model *Model) error {
if barResp == nil {
return fmt.Errorf("response input is nil")
}
if barResp.Bar == nil {
return fmt.Errorf("response bar is nil")
}
if model == nil {
return fmt.Errorf("model input is nil")
}

@Fyusel

Fyusel commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Hi @paul-sffrth,
the acceptance tests are currently failing. Both with this message:

 modelexperiments_acc_test.go:84: Step 1/4 error: Error running apply: exit status 1
        
        Error: Error creating AI Model Experiments instance
        
          with stackit_modelexperiments_instance.example,
          on terraform_plugin_test.tf line 25, in resource "stackit_modelexperiments_instance" "example":
          25: resource "stackit_modelexperiments_instance" "example" {
        
        Waiting for instance to be active: found non-GenericOpenApiError: waiting
        failed. state is impaired
        Trace ID: "54a07214967fd8f9381e2e314c9d8f49"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants