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

Add support for using a local now.json #4

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

Conversation

one000mph
Copy link
Contributor

@one000mph one000mph commented Jul 10, 2018

  1. Checks for local_config parameter (usually now.json in the pipeline step. If it exists, it uses the options set in the file and ignores any duplicate parameters in the pipeline step (e.g. if "name" in now.json and deploy_name: in .drone.yml are both set it uses "name" from now.json)
  2. If "alias" is set in now.json the plugin runs the alias command
  3. If "scale" is set in now.json the plugin runs the scale command
  4. If local_config is not set, it functions just the same

Other Notes

  • Changed code style
if <condition>
then
    <block>
fi

to if <condition>; then

  • Update to DOCS.md
  • Clarify how to use scale: parameter in docs
  • The full list of config features are documented here. Additional features may be used in the now.json that are not accessible with the current options in the plugin.

@lucaperret
Copy link
Owner

Hi @one000mph,
I'm not sure, but does the parameters of now override the now.json config ?

@@ -2,7 +2,7 @@ pipeline:
docker:
image: plugins/docker
secrets: [ docker_username, docker_password ]
repo: lucap/drone-now
repo: andyet/drone-now
Copy link
Owner

Choose a reason for hiding this comment

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

could you remove this change ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack yes, sorry I always forget

.gitignore Outdated
@@ -0,0 +1 @@
test.sh
Copy link
Owner

Choose a reason for hiding this comment

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

this is not included in your commit

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 did not commit it intentionally. It's a very short script that sets some environment vars and runs script.sh. It allows me to test on our org but contains a token so I did not commit it. I removed from the branch.

@one000mph
Copy link
Contributor Author

one000mph commented Jul 16, 2018

If you set flags on the CLI e.g. now -n my-deploy-name -A now.json and have

# now.json
{
	"name": "a-different-name"
}

now will use the name set by -n "my-deploy-name". Running now on the CLI, however, does not mirror the behavior of this plugin. I tried to make that clear in the Docs file. If you examine the plugin code, you will see that the command run when local_config is set does not include a -n flag. If you compare L34 https://github.com/andyet/drone-now/blob/8660b392d44316ff90d4a2338c5c46f6f2682333/script.sh#L34 with L54 https://github.com/andyet/drone-now/blob/8660b392d44316ff90d4a2338c5c46f6f2682333/script.sh#L54 you will see that difference. I'd welcome any other feedback that you have on the code. Right now the plugin assumes that if you set local_config all parameters which can be set in now.json are set there including the name of the deployment, alias, scale parameters, etc. It will ignore a duplicate parameter in the drone config if it exists. If you have additional feedback on my code, or suggestions on how to make this behavior more clear please let me know.

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.

2 participants