这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@michaelshobbs
Copy link
Member

uses dokku config:set to store first Dockerfile port for use in deploy phase

closes #993

Copy link
Member

Choose a reason for hiding this comment

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

Won't this trigger a ps:restart?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. I didn't see any issue because we guard against it on the first deploy.

https://github.com/progrium/dokku/blob/53f9974f6f905de5ec99163cc691f2493e6048e3/plugins/ps/commands#L86-L87

Let me rework the config portion.

@michaelshobbs
Copy link
Member Author

I'd kind of rather have an internal-only command that does this instead exposing it to the user. For two reasons.

  1. The option is not Heroku compliant
  2. Avoids more arg parsing code

@josegonzalez
Copy link
Member

Thats fine with me, something in common? That way plugins could still use it, but not users.

@josegonzalez
Copy link
Member

My concern is that adding something like this to common exposes configuration management outside of the config plugin, which I don't think is so great.

@michaelshobbs
Copy link
Member Author

How about dokku config:nset or something similar that wraps config:set but bypasses the restart?

@josegonzalez
Copy link
Member

Well it wouldn't be able to wrap config:set but would be wrapped. We can omit it from the help output so and document it in the development guide for plugins :)

@michaelshobbs
Copy link
Member Author

Works for me.

@michaelshobbs
Copy link
Member Author

ping @josegonzalez

@michaelshobbs michaelshobbs force-pushed the 993_mh-inspect-port-always branch from c72a229 to 767b45a Compare March 19, 2015 15:09
Copy link
Member

Choose a reason for hiding this comment

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

Much nicer! I wonder if there are other places we can use this pattern?

@josegonzalez
Copy link
Member

Merge once this passes :) Good work!

josegonzalez added a commit that referenced this pull request Mar 19, 2015
extract first port from Dockerfile and set config variable for use in deploy phase. closes #993
@josegonzalez josegonzalez merged commit 3caec44 into master Mar 19, 2015
@josegonzalez josegonzalez deleted the 993_mh-inspect-port-always branch March 19, 2015 17:54
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.

Allow to take internal port from a Dockerfile

3 participants