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

Make it possible to use relative links to recipe in workspace environment #3002

Merged
merged 10 commits into from
Nov 10, 2016

Conversation

mshaposhnik
Copy link
Contributor

@mshaposhnik mshaposhnik commented Nov 7, 2016

What does this PR do?

To ease data migration between domains, we should store local recipe links in a relative form. And when necessary, downloader should place current domain by itself.

What issues does this PR fix or reference?

Fixes #2769

@codenvy-ci
Copy link

@skabashnyuk skabashnyuk added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. sprint/current team/platform labels Nov 8, 2016
targetUriBuilder.scheme(apiEndpoint.getScheme())
.host(apiEndpoint.getHost())
.port(apiEndpoint.getPort())
.replacePath(apiEndpoint.getPath() + location);

Choose a reason for hiding this comment

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

Is it possible that api endpoint doesn't have trailing slash? In that case I suppose it would be better to use .replacePath().path()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK api endpoint never contain trailing slash

Choose a reason for hiding this comment

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

ok

targetUriBuilder.scheme(apiEndpoint.getScheme())
.host(apiEndpoint.getHost())
.port(apiEndpoint.getPort())
.replacePath(apiEndpoint.getPath() + location);

Choose a reason for hiding this comment

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

See above

@@ -298,7 +296,7 @@ public void shouldSetDockerfileContentInsteadOfUrlIfUrlPointsToCheApiOnEnvironme
// given
EnvironmentImpl env = createEnv();
String machineName = "machineWithDockerfileFromApi";
String additionalServiceComposeFilePart = "\n " + machineName + ":\n build:\n context: " + API_ENDPOINT + "/recipe/12345";

Choose a reason for hiding this comment

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

I assume that your changes should not influence these tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I's due to regexp change in CheEnvironmentEngine.

@codenvy-ci
Copy link

@codenvy-ci
Copy link

@skabashnyuk skabashnyuk added this to the 5.0.0-M8 milestone Nov 10, 2016
@mshaposhnik mshaposhnik merged commit f1c29d5 into master Nov 10, 2016
@mshaposhnik mshaposhnik deleted the CHE-2769-2 branch November 10, 2016 14:56
JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Nov 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed. sprint/current
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use relative link to recipe in workspace environment if possible
6 participants