-
Notifications
You must be signed in to change notification settings - Fork 9
[CFX-4801] Detect template using answers or cli directories
#341
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
answers or cli directories
bebdec5 to
632ae09
Compare
632ae09 to
6007c6f
Compare
|
🚀 Smoke tests triggered! Running on Linux and Windows... |
|
🚀 Smoke tests triggered! Running on Linux and Windows... |
|
❌ Some smoke tests failed. ❌ Linux: cancelled |
|
🚀 Smoke tests triggered! Running on Linux and Windows... |
|
✅ All smoke tests passed! ✅ Linux: success |
bd61a7e to
08bc512
Compare
|
❌ Some smoke tests failed. ❌ Linux: cancelled |
3d4e81b to
04a69d9
Compare
04a69d9 to
1dcbb78
Compare
|
🚀 Smoke tests triggered! Running on Linux and Windows... |
|
✅ All smoke tests passed! ✅ Linux: success |
internal/repo/detect.go
Outdated
| if fsutil.DirExists(filepath.Join(dir, DataRobotTemplateDetectAnswersPath)) { | ||
| return true | ||
| } | ||
|
|
||
| entries, err := os.ReadDir(filepath.Join(dir, DataRobotTemplateDetectCliPath)) | ||
| if err != nil { | ||
| return false | ||
| } | ||
|
|
||
| // Older versions were incorrectly creating state.yaml file outside of template directories | ||
| // return true if any file other than state.yaml exists in .datarobot/cli | ||
| return slices.ContainsFunc(entries, func(entry os.DirEntry) bool { | ||
| return entry.Name() != "state.yaml" |
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.
Can we add debug logging here? It will be important for us to keep track of these things.
internal/repo/paths.go
Outdated
| DataRobotTemplateDetectPath = ".datarobot/answers" | ||
| // DataRobotTemplateDetectAnswersPath is the path to the answers folder relative to CWD | ||
| DataRobotTemplateDetectAnswersPath = ".datarobot/answers" | ||
| DataRobotTemplateDetectCliPath = ".datarobot/cli" |
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.
| DataRobotTemplateDetectCliPath = ".datarobot/cli" | |
| // path to the CLI folder relative to CWD; some older templates use this rather than answers | |
| DataRobotTemplateDetectCliPath = ".datarobot/cli" |
ajalon1
left a 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.
I understand where this is coming from but we need to make sure @shreyaag-dr is onboard with this for older template versions, so that she can update the PRD around template root detection.
978e1bf to
ae68b6a
Compare
ae68b6a to
27d5128
Compare
RATIONALE
CHANGES
PR Automation
Comment-Commands: Trigger CI by commenting on the PR:
/trigger-smoke-testor/trigger-test-smoke- Run smoke tests/trigger-install-testor/trigger-test-install- Run installation testsLabels: Apply labels to trigger workflows:
run-smoke-testsorgo- Run smoke tests on demand (only works for non-forked PRs)Important
For Forked PRs: If you're an external contributor, the
run-smoke-testslabel won't work. Only maintainers can trigger smoke tests on forked PRs by applying theapproved-for-smoke-testslabel after security review. Please comment requesting maintainer review if you need smoke tests to run.