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

in dev mode use agents from the repo same way as api server #1436

Merged
merged 3 commits into from
Dec 28, 2016

Conversation

riuvshin
Copy link

@riuvshin riuvshin commented Dec 27, 2016

fixes needed for #1431

Those changes affects ONLY dev mode.
to make dev mode consistent we have to pick agents (ws agent and terminal) same way as api server to containers from :/repo

@TylerJewell @benoitf please carefully review this I want to be sure I didn't missed anything


This change is Reviewable

# path to codenvy ws agent for development mode
$codenvy_development_ws_agent = getValue("WS_AGENT_ASSEMBLY","/path/to/codenvy_tomcat")
# path to codenvy terminal agent for development mode
$codenvy_development_terminal_agent = getValue("TERMINAL_AGENT_ASSEMBLY","/path/to/codenvy_tomcat")
Copy link
Contributor

Choose a reason for hiding this comment

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

Terminal is not related to tomcat server. Why it is on such path?

Copy link
Author

Choose a reason for hiding this comment

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

this is "default" path which never be used as we always set this in CLI but I agree i will update this to be more clear, thanks

@@ -174,6 +174,26 @@ cmd_config_post_action() {
log "docker volume create --name=codenvy-postgresql-volume >> \"${LOGS}\""
docker volume create --name=codenvy-postgresql-volume >> "${LOGS}"
fi

if [ "${CHE_DEVELOPMENT_MODE}" = "on" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should have single folder with agents instead of adding small peace of code in this script on addition of each agent?

Copy link
Author

Choose a reason for hiding this comment

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

in fact all agents will appear in single dir

Copy link
Contributor

@TylerJewell TylerJewell left a comment

Choose a reason for hiding this comment

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

I would prefer that we place the file names for the agents as variables. You have them twice in these files so if we don't do them as variables then we could create problems in the future. Otherwise what you did was clear and clean.

@riuvshin
Copy link
Author

@TylerJewell I've updated PR, can you please review changes, I changed some ENV vars names to be more clear.

@riuvshin riuvshin merged commit 942cc19 into master Dec 28, 2016
@riuvshin riuvshin deleted the cli_dev_mode_fix branch December 28, 2016 09:00
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.

None yet

3 participants