Skip to content
This repository has been archived by the owner on Jun 24, 2019. It is now read-only.

Docker + config + Fix ssl listener fixes #9

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ccgagnon
Copy link

@ccgagnon ccgagnon commented Aug 1, 2017

With all those commits the build and run of the docker image is without flaws.

supported anymore in the uabot repo. REmove hack from docker file. Now it
should work nicely.
https://coveord.atlassian.net/browse/DA-720
 supported anymore in the uabot repo. REmove hack from docker file. Now it
 should work nicely.
https://coveord.atlassian.net/browse/DA-720
@erocheleau
Copy link
Contributor

Why did you rename the vendor/ folder?

Copy link
Contributor

@erocheleau erocheleau left a comment

Choose a reason for hiding this comment

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

Why is the vendor directory moved/renamed?

I approve of the removal of the hack/visit.go strongly however 👍

@@ -26,12 +26,6 @@ func (builder *botConfigurationBuilder) WithLanguages(languages []string) *botCo
return builder
}

func (builder *botConfigurationBuilder) AllAnonymous() *botConfigurationBuilder {
Copy link
Contributor

Choose a reason for hiding this comment

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

You could keep the function even if you do not use it anymore?

Copy link
Author

Choose a reason for hiding this comment

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

No, it wouldn't build.

Copy link
Contributor

Choose a reason for hiding this comment

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

What was the error?
The existence of a function shouldn't prevent you from building :)

@@ -147,8 +147,10 @@ func validateConfig(config *explorerlib.Config) error {
func Stop(writter http.ResponseWriter, request *http.Request) {
Vars := mux.Vars(request)
id, _ := uuid.FromString(Vars["id"])
scenariolib.Info.Printf("Stopping worker")
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use Println(), Printf() is meant to be used with a format string.

Println will give you a new line every time.

@ccgagnon
Copy link
Author

ccgagnon commented Oct 3, 2017

@erocheleau I didn't do any rename/move. I don't have that in my local repo. ¯_(ツ)_/¯
Maybe something from a previous merge, but I would see it.

close(quitChannels[id])
delete(quitChannels, id)
scenariolib.Info.Printf("Worker stopped")
Copy link
Contributor

Choose a reason for hiding this comment

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

Println too

@@ -26,12 +26,6 @@ func (builder *botConfigurationBuilder) WithLanguages(languages []string) *botCo
return builder
}

func (builder *botConfigurationBuilder) AllAnonymous() *botConfigurationBuilder {
builder.config.AllowAnonymous = true
Copy link
Author

Choose a reason for hiding this comment

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

@erocheleau that prevented the build to be successful.
The Config object in uabot repo doesn't have the AllowAnonymous property anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, yes, change it so it sets the AnonymousThreshold float64 to 1 instead. And there is no longer a need to allowAnonymous.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just remove the line builder.config.AllowAnonymous = true and that should be it.

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

2 participants