Changes made for provisoners flag by rabelmervin · Pull Request #221 · score-spec/score-compose

rabelmervin · 2024-12-13T16:46:05Z

Initially the provisioners flag doesnt support local file and stdin. But now local and stdin added

mathieu-benoit · 2024-12-13T19:00:24Z

You need to sign your commits to have the DCO passing successfully on this PR:

To add your Signed-off-by line to every commit in this branch:
Ensure you have a local copy of your branch by checking out the pull request locally via command line.
In your local branch, run: git rebase HEAD~1 --signoff
Force push your changes to overwrite the branch: git push --force-with-lease origin new

rabelmervin · 2024-12-14T16:42:01Z

Hello @mathieu-benoit @astromechza I have signed the commits and DCO is passed successfully :) Is there anything i can help you with ?

7h3-3mp7y-m4n · 2024-12-14T21:35:24Z

hey, @rabelmervin I'm not a reviewer but can you also update the test case over here https://github.com/score-spec/score-compose/blob/main/internal/command/init_test.go#L79
Can you also add the test case for your function?

7h3-3mp7y-m4n · 2024-12-16T09:29:42Z

provisioner.yaml

@@ -0,0 +1,35 @@
+# Score provides a developer-centric and platform-agnostic 


I think @rabelmervin you should not add this file cause it's irrelevant to issue #220
Also, it is of no use.

7h3-3mp7y-m4n · 2024-12-16T09:32:53Z

score.yaml

+# Score provides a developer-centric and platform-agnostic 
+# Workload specification to improve developer productivity and experience. 
+# Score eliminates configuration management between local and remote environments.
+#
+# Specification reference: https://docs.score.dev/docs/reference/score-spec-reference/
+---
+
+# Score specification version
+apiVersion: score.dev/v1b1
+
+metadata:
+  name: example
+
+containers:
+  hello-world:
+    image: nginx:latest
+
+    # Uncomment the following for a custom entrypoint command
+    # command: []
+
+    # Uncomment the following for custom arguments
+    # args: []
+
+    # Environment variables to inject into the container
+    variables:
+      EXAMPLE_VARIABLE: "example-value"
+
+service:
+  ports:
+    # Expose the http port from nginx on port 8080
+    www:
+      port: 8080
+      targetPort: 80
+
+resources: {}


Same for this, If you want to add examples, you should refer to this and in a separate PR.

rabelmervin · 2024-12-16T10:00:53Z

Hi @7h3-3mp7y-m4n I'll the add the test functions and I'll remove that irrelevant files 👍

internal/command/default.provisioners.yaml

astromechza · 2025-01-06T11:00:16Z

internal/command/init.go

+
+				var saveFilename string
+				if vi == "-" {
+					saveFilename = fmt.Sprintf("stdin-provisioner-%d%s", i+1, loader.DefaultSuffix)


The file name here needs to look like a provisioners file. I'd also suggest dropping the %d suffix since we can only ever have one stdin file.

Suggested change

saveFilename = fmt.Sprintf("stdin-provisioner-%d%s", i+1, loader.DefaultSuffix)

saveFilename = "from-stdin.provisioners.yaml"

sure @astromechza @mathieu-benoit I'll make changes !. Is there anything i can help you with ?

astromechza · 2025-01-06T11:03:00Z

internal/command/init.go

 	"github.com/score-spec/score-compose/internal/provisioners/loader"
 )

+func GetStdinFile(ctx context.Context) ([]byte, error) {


Could we please move this functionality over to https://github.com/score-spec/score-go/blob/main/uriget/uriget.go#L155 as part of the file type getter? This is a good place to add unit tests too.

This is common functionality that we can use and document in all the consumers of this library. This would allow the uri to be documented as either: file://- or just -.

I can help get this turned around quickly :)

rabelmervin · 2025-01-17T04:23:50Z

Hi @mathieu-benoit and @astromechza Now the Test is passed. I'll commit the changes What's your opinion ?

astromechza · 2025-01-18T16:28:19Z

@rabelmervin Thanks! can you address the remaining comments on the diff, including this one for score-go? https://github.com/score-spec/score-compose/pull/221/files#r1904017231

rabelmervin · 2025-01-27T07:32:49Z

Bonjour @mathieu-benoit @astromechza I made changes ! I have Moved the GetStdinFile functionality to uriget.go present in score-go as part of GetFile score-spec/score-go#85

Signed-off-by: rabelmervin <rabelmervin@gmail.com>

7h3-3mp7y-m4n · 2025-02-02T11:06:28Z

I have proposed some changes to your score-go PR
Please wait for @astromechza or @mathieu-benoit to review it, Thanks!

astromechza · 2025-02-17T13:46:35Z

@rabelmervin your score-go PR is now released under https://github.com/score-spec/score-go/releases/tag/v1.9.2 and can be imported here :)

mathieu-benoit · 2025-03-06T14:36:09Z

Hi @rabelmervin, while we were reviewing the projects status in preparation of KubeCon, we were wondering if you could continue on this one? Thanks again for your contributions!

rabelmervin · 2025-03-06T15:45:14Z

Ohh ! I forgot this, Sure I'll complete this @mathieu-benoit @astromechza but, could you please tell me what yet to be addressed in this pr ?

Signed-off-by: Rabel Mervin <152761588+rabelmervin@users.noreply.github.com>

astromechza

There are a few remaining bits to clean up here :) @rabelmervin

astromechza · 2025-03-07T23:00:26Z

internal/command/init.go

+				var data []byte
+
+				if vi == "-" {
+					data, err = uriget.GetFile(cmd.Context(), "-")
+				} else {
+					// Existing URI loading logic
+					data, err = uriget.GetFile(cmd.Context(), vi)
+				}
+


None of this is necessary due to line 211 below. it's exactly the same.

go.mod

astromechza · 2025-03-07T23:02:35Z

internal/command/default.provisioners.yaml

        timeout: 10s
        retries: 120
-  info_logs: |
+    info_logs: |


This is incorrect, the info logs section needs to be indented only 2 spaces, it's a top level field.

In fact I don't think any changes are necessary to this file?

sure, I'll not make any changes to this

Signed-off-by: rabelmervin <rabelmervin@gmail.com>

rabelmervin · 2025-03-08T16:00:48Z

Hello @astromechza i have made some changes.

Signed-off-by: Ben Meier <1651305+astromechza@users.noreply.github.com>

astromechza

Thanks, @rabelmervin, for your contribution and working through the feedback. I really appreciate it. :)

rabelmervin · 2025-03-10T17:18:44Z

Hurray, I'm over the moon thanks @astromechza and @mathieu-benoit Merci beaucoup.

mathieu-benoit linked an issue Dec 13, 2024 that may be closed by this pull request

[bug] score-compose init --provisioners flag doesn't document local file option and doesn't support stdin #220

Closed

mathieu-benoit requested a review from astromechza December 13, 2024 19:01

rabelmervin force-pushed the new branch from d1bff0d to d55e66d Compare December 14, 2024 16:29

7h3-3mp7y-m4n reviewed Dec 16, 2024

View reviewed changes

astromechza requested changes Jan 6, 2025

View reviewed changes

rabelmervin added 5 commits February 1, 2025 22:41

changes made

e7b1d92

Signed-off-by: rabelmervin <rabelmervin@gmail.com>

%d dropped and changes made

24646a9

Signed-off-by: rabelmervin <rabelmervin@gmail.com>

TestInitHelp updated

fb9c710

Signed-off-by: rabelmervin <rabelmervin@gmail.com>

Functionality getstdinfile removed

7924780

Signed-off-by: rabelmervin <rabelmervin@gmail.com>

irrelevant files removed

204694e

Signed-off-by: rabelmervin <rabelmervin@gmail.com>

rabelmervin force-pushed the new branch from 6d20f1a to 204694e Compare February 1, 2025 17:11

rabelmervin requested review from 7h3-3mp7y-m4n and astromechza February 1, 2025 17:14

astromechza added the stale label Mar 6, 2025

Merge branch 'main' into new

7dcd7e6

Signed-off-by: Rabel Mervin <152761588+rabelmervin@users.noreply.github.com>

astromechza requested changes Mar 7, 2025

View reviewed changes

rabelmervin and others added 2 commits March 8, 2025 13:46

Merge branch 'score-spec:main' into new

1a64c80

Changes

c690aba

Signed-off-by: rabelmervin <rabelmervin@gmail.com>

rabelmervin requested a review from astromechza March 8, 2025 16:01

Update default.provisioners.yaml

d424e38

Signed-off-by: Ben Meier <1651305+astromechza@users.noreply.github.com>

astromechza approved these changes Mar 10, 2025

View reviewed changes

astromechza merged commit ecd9290 into score-spec:main Mar 10, 2025
4 checks passed

		@@ -0,0 +1,35 @@
		# Score provides a developer-centric and platform-agnostic

	saveFilename = fmt.Sprintf("stdin-provisioner-%d%s", i+1, loader.DefaultSuffix)
	saveFilename = "from-stdin.provisioners.yaml"

Conversation

rabelmervin commented Dec 13, 2024

Uh oh!

mathieu-benoit commented Dec 13, 2024

Uh oh!

rabelmervin commented Dec 14, 2024

Uh oh!

7h3-3mp7y-m4n commented Dec 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

7h3-3mp7y-m4n Dec 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

7h3-3mp7y-m4n Dec 16, 2024

Choose a reason for hiding this comment

Uh oh!

rabelmervin commented Dec 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

astromechza Jan 6, 2025

Choose a reason for hiding this comment

Uh oh!

rabelmervin Jan 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astromechza Jan 6, 2025

Choose a reason for hiding this comment

Uh oh!

rabelmervin commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

astromechza commented Jan 18, 2025

Uh oh!

rabelmervin commented Jan 27, 2025

Uh oh!

7h3-3mp7y-m4n commented Feb 2, 2025

Uh oh!

astromechza commented Feb 17, 2025

Uh oh!

mathieu-benoit commented Mar 6, 2025

Uh oh!

rabelmervin commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

astromechza left a comment

Choose a reason for hiding this comment

Uh oh!

astromechza Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

astromechza Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

astromechza Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

rabelmervin Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

rabelmervin commented Mar 8, 2025

Uh oh!

astromechza left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rabelmervin commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

7h3-3mp7y-m4n commented Dec 14, 2024 •

edited

Loading

7h3-3mp7y-m4n Dec 16, 2024 •

edited

Loading

rabelmervin commented Dec 16, 2024 •

edited

Loading

rabelmervin Jan 7, 2025 •

edited

Loading

rabelmervin commented Jan 17, 2025 •

edited

Loading

rabelmervin commented Mar 6, 2025 •

edited

Loading

rabelmervin commented Mar 10, 2025 •

edited

Loading