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

Remove all lines between Builder & Depot #3239

Merged
merged 2 commits into from Sep 20, 2017

Conversation

@reset
Copy link
Member

commented Sep 20, 2017

  • Hab CLI has had it's documentation updated to reference Builder
    instead of the Depot. This is primarily present in any commands
    which accept a --url and/or a --channel flag
  • HAB_DEPOT_URL renamed to HAB_BLDR_URL. We will still honor
    HAB_DEPOT_URL but it is no longer present in documentation
  • HAB_DEPOT_CHANNEL renamed to HAB_BLDR_CHANNEL. This is
    still supported the same as HAB_DEPOT_URL
  • The value of --url or HAB_BLDR_URL no longer expects a path
    portion of the URL. For example, you no longer need to specify
    --url http://localhost:9636/v1/depot. This was leaking the API
    version and it's API in the CLI causing issues for clients which
    were to be configured to speak to a non-public Builder endpoint
  • Large portions of the documentation have been updated to reference
    Builder as an entity. Documentation about the "Depot component"
    has not been altered but it should be given a refactor
  • Builder-Web configuration no longer expects a url containing a
    path. This is identicle to the changes to the value of --url on
    in the CLI
  • Renamed "Depot" to "Builder" on Builder-Web
  • Removed the unused and unmaintained ruby-client from components

tenor-69835060

@thesentinels

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2017

Thanks for the pull request! Here is what will happen next:

  1. Your PR will be reviewed by the maintainers
  2. If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

Remove all lines between Builder & Depot
* Hab CLI has had it's documentation updated to reference Builder
  instead of the Depot. This is primarily present in any commands
  which accept a `--url` and/or a `--channel` flag
* `HAB_DEPOT_URL` renamed to `HAB_BLDR_URL`. We will still honor
  `HAB_DEPOT_URL` but it is no longer present in documentation
* `HAB_DEPOT_CHANNEL` renamed to `HAB_BLDR_CHANNEL`. This is
  still supported the same as `HAB_DEPOT_URL`
* The value of `--url` or `HAB_BLDR_URL` no longer expects a path
  portion of the URL. For example, you no longer need to specify
  `--url http://localhost:9636/v1/depot`. This was leaking the API
  version and it's API in the CLI causing issues for clients which
  were to be configured to speak to a non-public Builder endpoint
* Large portions of the documentation have been updated to reference
  Builder as an entity. Documentation about the "Depot component"
  has not been altered but it should be given a refactor
* Builder-Web configuration no longer expects a url containing a
  path. This is identicle to the changes to the value of `--url` on
  in the CLI
* Renamed "Depot" to "Builder" on Builder-Web

Signed-off-by: Jamie Winsor <jamie@vialstudios.com>

@reset reset force-pushed the depot-bldr branch from 1f53a88 to 089b06c Sep 20, 2017

@elliott-davis
Copy link
Contributor

left a comment

Web changes look good
tenor-12163494

Remove unused config from scheduler & rename Depot in Worker
* Remove unused `depot_url` config value from ScheduleSrv
* Rename `depot_url` and `depot_channel` config values in Worker
  to `bldr_url` and `bldr_channel`

Signed-off-by: Jamie Winsor <jamie@vialstudios.com>
@mgamini
Copy link
Contributor

left a comment

We can discuss protocol / url stuff later.

@@ -19,7 +19,7 @@ import { GitHubApiClient } from "./GitHubApiClient";

export class BuilderApiClient {
private headers;
private urlPrefix: string = config["habitat_api_url"];
private urlPrefix: string = `${config["habitat_api_url"]}/v1`;

This comment has been minimized.

Copy link
@mgamini

mgamini Sep 20, 2017

Contributor

Maybe /v1 should just be in the config?

This comment has been minimized.

Copy link
@reset

reset Sep 20, 2017

Author Member

It's on purpose because this doesn't belong in the config. This is part of the "protocol" and not part of the configuration. The client should hardcode which protocol it expects as it is not actually configurable.

For example, say we had added a v2. Well, I could configure this client to talk to v2 but it would have absolutely no idea how to handle it :)

@@ -4,7 +4,7 @@ habitatConfig({
// of the habitat repo, this will be localhost (if running Docker for Mac or
// Linux) or the result of `$(docker-machine ip default)` if using Docker
// in a virtual Machine.
habitat_api_url: "http://localhost:9636/v1",
habitat_api_url: "http://localhost:9636",

This comment has been minimized.

Copy link
@mgamini

mgamini Sep 20, 2017

Contributor

Oh. This was on purpose. Why?

@reset

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2017

@fnichol per our conversation on the walk out of Rakuten, take a peek at this post-merge for me. I'm going to get this in now so I can get moving on testing in acceptance!

@reset reset merged commit 4743690 into master Sep 20, 2017

3 checks passed

DCO This commit has a DCO Signed-off-by line
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@reset reset deleted the depot-bldr branch Sep 20, 2017

(@arg DEPOT_URL: -u --url +takes_value {valid_url}
"Use a specific Depot URL (ex: http://depot.example.com/v1/depot)")
(@arg BLDR_URL: -u --url +takes_value {valid_url}
"Specify an alternate Builder endpoint (ex: https://bldr.habitat.sh)")

This comment has been minimized.

Copy link
@fnichol

fnichol Sep 20, 2017

Member

Maybe change the (ex: https://bldr.habitat.sh) to (default: https://bldr.habitat.sh)?

This comment has been minimized.

Copy link
@reset

reset Sep 20, 2017

Author Member

Gonna get this and the others in the next one, great idea

(@arg AUTH_TOKEN: -z --auth +takes_value "Authentication token for the Depot")
(@arg GROUP: -g --group "Schedule jobs for this package and all of its reverse dependencies")
(@arg BLDR_URL: -u --url +takes_value {valid_url}
"Specify an alternate Builder endpoint (ex: https://bldr.habitat.sh)")

This comment has been minimized.

Copy link
@fnichol

fnichol Sep 20, 2017

Member

(default: https://bldr.habitat.sh)?

(@arg CHANNEL: +takes_value +required "The channel name to promote the built \
packages into")
(@arg BLDR_URL: -u --url +takes_value {valid_url}
"Specify an alternate Builder endpoint (ex: https://bldr.habitat.sh)")

This comment has been minimized.

Copy link
@fnichol

fnichol Sep 20, 2017

Member

(default: https://bldr.habitat.sh)?

(@arg DEPOT_URL: -u --url +takes_value {valid_url}
"Use a specific Depot URL (ex: http://depot.example.com/v1/depot)")
(@arg BLDR_URL: -u --url +takes_value {valid_url}
"Specify an alternate Builder endpoint (ex: https://bldr.habitat.sh)")

This comment has been minimized.

Copy link
@fnichol

fnichol Sep 20, 2017

Member

(default: https://bldr.habitat.sh)?

"Use a specific Depot URL (ex: http://depot.example.com/v1/depot)")
(@arg AUTH_TOKEN: -z --auth +takes_value "Authentication token for the Depot")
(@arg BLDR_URL: -u --url +takes_value {valid_url}
"Specify an alternate Builder endpoint (ex: https://bldr.habitat.sh)")

This comment has been minimized.

Copy link
@fnichol

fnichol Sep 20, 2017

Member

(default: https://bldr.habitat.sh)?

"Use a specific Depot URL (ex: http://depot.example.com/v1/depot)")
(@arg AUTH_TOKEN: -z --auth +takes_value "Authentication token for the Depot")
(@arg BLDR_URL: -u --url +takes_value {valid_url}
"Specify an alternate Builder endpoint (ex: https://bldr.habitat.sh)")

This comment has been minimized.

Copy link
@fnichol

fnichol Sep 20, 2017

Member

(default: https://bldr.habitat.sh)?

(@arg DEPOT_URL: -u --url +takes_value {valid_url}
"Use a specific Depot URL (ex: http://depot.example.com/v1/depot)")
(@arg BLDR_URL: -u --url +takes_value {valid_url}
"Specify an alternate Builder endpoint (ex: https://bldr.habitat.sh)")

This comment has been minimized.

Copy link
@fnichol

fnichol Sep 20, 2017

Member

(default: https://bldr.habitat.sh)?

(@arg DEPOT_URL: -u --url +takes_value {valid_url}
"Use a specific Depot URL (ex: http://depot.example.com/v1/depot)")
(@arg BLDR_URL: -u --url +takes_value {valid_url}
"Specify an alternate Builder endpoint (ex: https://bldr.habitat.sh)")

This comment has been minimized.

Copy link
@fnichol

fnichol Sep 20, 2017

Member

(default: https://bldr.habitat.sh)?

@@ -1,6 +1,6 @@
ident = "sup-integration-test/config-changes-hooks-do-not"
group = "default"
depot_url = "http://hab.sup.test/v1/depot"

This comment has been minimized.

Copy link
@fnichol

fnichol Sep 20, 2017

Member

depot_url -> bldr_url?

This comment has been minimized.

Copy link
@reset

reset Sep 20, 2017

Author Member

Got it in a future one

@@ -113,7 +113,7 @@ In this stage, we rebuild all the base packages needed by Habitat using the tool
export STUDIO_TYPE=default
export HAB_STUDIO_ROOT=/hab/studios/stage2
export HAB_ORIGIN=core
export HAB_DEPOT_URL=http://127.0.0.1:9632/v1
export HAB_BLDR_URL=http://127.0.0.1:9632/v1

This comment has been minimized.

Copy link
@fnichol

fnichol Sep 20, 2017

Member

Drop the /v1?

This comment has been minimized.

Copy link
@reset

reset Sep 20, 2017

Author Member

Holy crap, good catch. These are ancient docs, that port doesn't even get used anymore :). I'll get this in the next one

@fnichol

This comment has been minimized.

Copy link
Member

commented Sep 20, 2017

That’s looking pretty complete to me

@raskchanky raskchanky referenced this pull request Sep 25, 2017

Closed

Default API URL no longer handles `/depot` suffix #3313

0 of 7 tasks complete

@tduffield tduffield referenced this pull request Sep 25, 2017

Merged

Fix for hab 0.33.2 - do not provide '/v1/depot' to Hab URL #46

0 of 4 tasks complete

fnichol added a commit that referenced this pull request Sep 29, 2017

[Builder] Drop `depot` bind for scheduler in development container.
References #3239

Signed-off-by: Fletcher Nichol <fnichol@nichol.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.