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: implement update script and build/push workflow #102

Merged
merged 50 commits into from
Mar 11, 2024

Conversation

ayildirim21
Copy link
Member

Improve the CI of SDK as per #1363

@ayildirim21 ayildirim21 marked this pull request as draft March 6, 2024 19:23
@KeranYang KeranYang changed the title Implement update script and build/push workflow feat: implement update script and build/push workflow Mar 7, 2024
Copy link
Member

@KeranYang KeranYang left a comment

Choose a reason for hiding this comment

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

Thanks for making the change. I left some comments. Also, how was this change tested? and can we fix the snyk security scan failure?

.github/workflows/buid-push.yaml Outdated Show resolved Hide resolved
.github/workflows/buid-push.yaml Outdated Show resolved Hide resolved
update_examples.sh Outdated Show resolved Hide resolved
update_examples.sh Show resolved Hide resolved
@vigith
Copy link
Member

vigith commented Mar 7, 2024

you can set it to v0.0.0

replace github.com/numaproj/numaflow-go => ../../../../../numaflow-go

require github.com/numaproj/numaflow-go v0.0.0

@ayildirim21
Copy link
Member Author

you can set it to v0.0.0

replace github.com/numaproj/numaflow-go => ../../../../../numaflow-go

require github.com/numaproj/numaflow-go v0.0.0

If we set to v0.0.0 then in the logs when we print out server info the version that is shown will also be v0.0.0. Would it be better if developers use the script to update the version when they want so it is accurately reflected in the logs?

@ayildirim21 ayildirim21 force-pushed the improve-ci-1363 branch 2 times, most recently from 57ae74e to 18f636c Compare March 7, 2024 17:53
@ayildirim21
Copy link
Member Author

ayildirim21 commented Mar 7, 2024

Thanks for making the change. I left some comments. Also, how was this change tested? and can we fix the synk security scan failure?

I dont have access to snyk so cannot tell why the failures are occuring, but im pretty sure it has something to do with the fact that we modified the go.mod/go.sum files

In terms of testing, I tested the script locally and it seems to be working as expected. For the workflow once we add the repo secrets (quay.io username and password) i will change the workflow to be triggered on push to see if the images are built and pushed correctly.

@vigith
Copy link
Member

vigith commented Mar 7, 2024

If we set to v0.0.0 then in the logs when we print out server info the version that is shown will also be v0.0.0. Would it be better if developers use the script to update the version when they want so it is accurately reflected in the logs?

ah, server-info will have the wrong version. good point :)

Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
Signed-off-by: a3hadi <[email protected]>
@KeranYang KeranYang marked this pull request as ready for review March 11, 2024 14:39
development.md Outdated Show resolved Hide resolved
@KeranYang KeranYang merged commit ec242ee into numaproj:main Mar 11, 2024
2 of 3 checks passed
ayildirim21 added a commit to ayildirim21/numaflow-go that referenced this pull request Mar 12, 2024
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.

3 participants