Skip to content
This repository has been archived by the owner on Jan 17, 2022. It is now read-only.

Fix/uppsf 666 docker image update #23

Merged
merged 3 commits into from
May 20, 2020

Conversation

gkazakov111
Copy link
Contributor

No description provided.

@gkazakov111 gkazakov111 requested review from a team April 10, 2020 11:41
@mitagg
Copy link

mitagg commented Apr 10, 2020

Please change the content in the PanicGuide with some relevant information.

.circleci/config.yml Outdated Show resolved Hide resolved
Copy link

@karsov karsov left a comment

Choose a reason for hiding this comment

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

The Dockerfile does not look OK.
The service is not released to dev and therefore not tested if it works.
Let's also add a "__build-info" endpoint. I think we should add the following line after line 96 in "synth-publish.go"

http.HandleFunc(httphandlers.BuildInfoPath, httphandlers.BuildInfoHandler)

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@gkazakov111 gkazakov111 force-pushed the fix/UPPSF-666-docker-image-update branch from 03a3a5a to 2ce14ba Compare April 14, 2020 10:05
@mitagg
Copy link

mitagg commented Apr 14, 2020

We can also add a badge for the go report card

@mitagg
Copy link

mitagg commented Apr 14, 2020

@gkazakov111 with the current Docker file the start.sh script won't find the required files.

@gkazakov111 gkazakov111 force-pushed the fix/UPPSF-666-docker-image-update branch 2 times, most recently from e6a6768 to 5ace2a0 Compare April 15, 2020 11:21
Dockerfile Outdated Show resolved Hide resolved
@mitagg mitagg self-requested a review April 28, 2020 09:04
mitagg
mitagg previously approved these changes Apr 28, 2020
Copy link

@MiroslavGatsanoga MiroslavGatsanoga left a comment

Choose a reason for hiding this comment

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

Please squash the commits before merging.

.circleci/config.yml Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
karsov
karsov previously requested changes May 15, 2020
Copy link

@karsov karsov left a comment

Choose a reason for hiding this comment

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

synthetic-image-publication-monitor-6b486d58b7-rts7v synthetic-image-publication-monitor 2020/05/15 13:40:58 cmdR.Run() failed with exec: "kubectl": executable file not found in $PATH

Dockerfile Outdated Show resolved Hide resolved
@gkazakov111 gkazakov111 force-pushed the fix/UPPSF-666-docker-image-update branch from b8829b2 to 5a4fc6d Compare May 15, 2020 13:50
@karsov karsov dismissed their stale review May 15, 2020 15:29

Updated

@karsov
Copy link

karsov commented May 15, 2020

Seems to be OK now, however I'd recommend to watch for some time to see if the kubectl logic works.
I can't brake the publish in a way that triggers it ( https://github.com/Financial-Times/coco-synthetic-image-publication-monitor/blob/master/synth-publish.go#L236 )

@gkazakov111 gkazakov111 force-pushed the fix/UPPSF-666-docker-image-update branch from 5a4fc6d to 7762477 Compare May 19, 2020 12:38
@gkazakov111 gkazakov111 removed the request for review from a team May 20, 2020 07:38
@gkazakov111 gkazakov111 merged commit a3a20cc into master May 20, 2020
@gkazakov111 gkazakov111 deleted the fix/UPPSF-666-docker-image-update branch May 20, 2020 08:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants