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

Allow to provides connection through command line #4

Closed
laedit opened this Issue Oct 7, 2016 · 12 comments

Comments

Projects
None yet
2 participants
@laedit

laedit commented Oct 7, 2016

I plan to use creep in a continuous delivery context, with the files generated during build on the CI server.
For example for my ReadingList project, I run a F# script on AppVeyor which generates some content and after I deploy the files on my FTP.

So right now, if I want to use creep to deploy the files, I need to provides the .creep.env and .creep.def files, either by having them on git (but with the FTP password that's not really an option) or by generated them, which is not really practical.

I was thinking to add more arguments to creep, like connection, local, source and maybe algorithm and follow.

The goal is not to replace the files, but more to allow a minimal use of creep without using any file.

If you are ok with this idea I will make a PR, tested on both Python 2 & 3 this time 😄

@r3c

This comment has been minimized.

Show comment
Hide comment
@r3c

r3c Oct 7, 2016

Owner

I just have a few questions to be sure I understand your requirement: is your concern about having password stored in the "env" file, or do you want to avoid having files at all and specify everything through command line?

For the first problem I was thinking about some inclusion mechanism so password could be moved out of env file, but maybe that doesn't solve your problem at all.

Your idea about specifying options though command line is nice, the only drawback I see is that it requires some duplication between configuration file reading and command line parsing (each time a new option is added it must be added twice). Maybe this could be solved by accepting a JSON-serialized string in command line so that same deserialization mechanism can be used (e.g. --env '{"default": {"connection": "..."}}'). It would be a bit ugly but would remove the duplication.

What do you think?

Owner

r3c commented Oct 7, 2016

I just have a few questions to be sure I understand your requirement: is your concern about having password stored in the "env" file, or do you want to avoid having files at all and specify everything through command line?

For the first problem I was thinking about some inclusion mechanism so password could be moved out of env file, but maybe that doesn't solve your problem at all.

Your idea about specifying options though command line is nice, the only drawback I see is that it requires some duplication between configuration file reading and command line parsing (each time a new option is added it must be added twice). Maybe this could be solved by accepting a JSON-serialized string in command line so that same deserialization mechanism can be used (e.g. --env '{"default": {"connection": "..."}}'). It would be a bit ugly but would remove the duplication.

What do you think?

@laedit

This comment has been minimized.

Show comment
Hide comment
@laedit

laedit Oct 7, 2016

Sorry, I wasn't enough specific, I have concerns about either the password in file and the files themselves.
The first for security reason, the second because since all files of the website are generated, I need to also generate or copy the .creep.env and .creep.def files and I prefer to avoid that if possible.

To avoid the duplication, what about adding create_environment and create_definition methods in factory, which will create environment and definition from args or file if it exists?

laedit commented Oct 7, 2016

Sorry, I wasn't enough specific, I have concerns about either the password in file and the files themselves.
The first for security reason, the second because since all files of the website are generated, I need to also generate or copy the .creep.env and .creep.def files and I prefer to avoid that if possible.

To avoid the duplication, what about adding create_environment and create_definition methods in factory, which will create environment and definition from args or file if it exists?

@r3c

This comment has been minimized.

Show comment
Hide comment
@r3c

r3c Oct 7, 2016

Owner

I think understand your concern, but not your last suggestion. Instead of passing new create_environment and create_definition options I suggested to directly use the data provided through command line without creating a file at all.

For example creep --environment-inline '{"default": {"connection": "ftp://someftp/"}}' default would use "ftp://someftp" as remote location without creating any environment file. Since this probably only make sense for environment files but not definition files (which can be versionned safely along with project source files as they are deployment-agnostic and don't contain any password) we could also simplify the command line a bit and directly accept extra "inline" anonymous deployment locations, e.g. creep --location '{"connection": "ftp://someftp/"}'.

Could that be an option in your case?

Owner

r3c commented Oct 7, 2016

I think understand your concern, but not your last suggestion. Instead of passing new create_environment and create_definition options I suggested to directly use the data provided through command line without creating a file at all.

For example creep --environment-inline '{"default": {"connection": "ftp://someftp/"}}' default would use "ftp://someftp" as remote location without creating any environment file. Since this probably only make sense for environment files but not definition files (which can be versionned safely along with project source files as they are deployment-agnostic and don't contain any password) we could also simplify the command line a bit and directly accept extra "inline" anonymous deployment locations, e.g. creep --location '{"connection": "ftp://someftp/"}'.

Could that be an option in your case?

@laedit

This comment has been minimized.

Show comment
Hide comment
@laedit

laedit Oct 8, 2016

I wasn't suggesting to create files from the command line arguments but rather to create environment and definition objects through new methods in the factory class, or elsewhere if you prefer.

If you prefer to stick to JSON in argument values I am ok with it, but if possible I's prefer if definition also had it's command line argument, since like all the files that I want to transfer through FTP are generated, I'd prefer not to store the .creep.def file and copy it in the generated folder before using creep.

Let me know If I am not clear enough 😄

laedit commented Oct 8, 2016

I wasn't suggesting to create files from the command line arguments but rather to create environment and definition objects through new methods in the factory class, or elsewhere if you prefer.

If you prefer to stick to JSON in argument values I am ok with it, but if possible I's prefer if definition also had it's command line argument, since like all the files that I want to transfer through FTP are generated, I'd prefer not to store the .creep.def file and copy it in the generated folder before using creep.

Let me know If I am not clear enough 😄

@r3c

This comment has been minimized.

Show comment
Hide comment
@r3c

r3c Oct 8, 2016

Owner

Oh OK sorry then I probably didn't understand exactly your suggestion. Could you please give me an example of what your new command line options would look like? (I'm trying to understand how you would transform them into environment and definitions objects without having to duplicate schema declaration somewhere 😄 )

Owner

r3c commented Oct 8, 2016

Oh OK sorry then I probably didn't understand exactly your suggestion. Could you please give me an example of what your new command line options would look like? (I'm trying to understand how you would transform them into environment and definitions objects without having to duplicate schema declaration somewhere 😄 )

@laedit

This comment has been minimized.

Show comment
Hide comment
@laedit

laedit Oct 8, 2016

Sorry about the confusion, I should have done that from the beginning 😞

Hare is what I have in mind, something like that:

creep --connection ftp://someftp/ --local --source hash

Then in creep.py:

environment = factory.create_environment (args)

And in factory.py or a dedicated file:

def create_environment (args):
    if args.connection != ""
        return Environment (args.connection, args.local)
    else
        return Environement (args.environment_path)

So this is just the general idea, I know that this is not usable as is.

laedit commented Oct 8, 2016

Sorry about the confusion, I should have done that from the beginning 😞

Hare is what I have in mind, something like that:

creep --connection ftp://someftp/ --local --source hash

Then in creep.py:

environment = factory.create_environment (args)

And in factory.py or a dedicated file:

def create_environment (args):
    if args.connection != ""
        return Environment (args.connection, args.local)
    else
        return Environement (args.environment_path)

So this is just the general idea, I know that this is not usable as is.

@r3c

This comment has been minimized.

Show comment
Hide comment
@r3c

r3c Oct 8, 2016

Owner

OK that's more or less what I had in mind, but I believe it has the problem I was trying to describe above: you expose separated parameters (--connection, --local, --source, etc.) through command-line meaning you'll have to duplicate their interpretation somewhere (even if both implementations are located in "factory" class, exactly like the if you show in your last snippet). This is what I tried to avoid by passing a single opaque JSON parameter and let existing code in factory parse it, but I admit it's not very nice.

I don't have better solution to suggest though ; let me dig a bit more to see if there is a way to get the best of both approaches 😄

Owner

r3c commented Oct 8, 2016

OK that's more or less what I had in mind, but I believe it has the problem I was trying to describe above: you expose separated parameters (--connection, --local, --source, etc.) through command-line meaning you'll have to duplicate their interpretation somewhere (even if both implementations are located in "factory" class, exactly like the if you show in your last snippet). This is what I tried to avoid by passing a single opaque JSON parameter and let existing code in factory parse it, but I admit it's not very nice.

I don't have better solution to suggest though ; let me dig a bit more to see if there is a way to get the best of both approaches 😄

@laedit

This comment has been minimized.

Show comment
Hide comment
@laedit

laedit Oct 8, 2016

If nothing works out, the JSON approach will do the thing.

laedit commented Oct 8, 2016

If nothing works out, the JSON approach will do the thing.

@r3c

This comment has been minimized.

Show comment
Hide comment
@r3c

r3c Oct 8, 2016

Owner

I just published a 0.3.4 version on PyPi with support for extended -d and -e arguments: you can now specify JSON directly instead of having to create file. I admit it's not as convenient as the solution you suggested but I'd really like to continue thinking about this and see how to avoid duplicating configuration parsing.

Hopefully this new version will unblock your project in the meantime! Thanks again for your ideas 👍

Owner

r3c commented Oct 8, 2016

I just published a 0.3.4 version on PyPi with support for extended -d and -e arguments: you can now specify JSON directly instead of having to create file. I admit it's not as convenient as the solution you suggested but I'd really like to continue thinking about this and see how to avoid duplicating configuration parsing.

Hopefully this new version will unblock your project in the meantime! Thanks again for your ideas 👍

@laedit

This comment has been minimized.

Show comment
Hide comment
@laedit

laedit Oct 8, 2016

Thanks, I will test it ASAP. 😃

laedit commented Oct 8, 2016

Thanks, I will test it ASAP. 😃

@laedit

This comment has been minimized.

Show comment
Hide comment
@laedit

laedit Oct 10, 2016

Everything works fine for me.
I admit it is not elegant but it solve my issue.

So you can close this or leave it open if you prefer to find a more convenient way.

laedit commented Oct 10, 2016

Everything works fine for me.
I admit it is not elegant but it solve my issue.

So you can close this or leave it open if you prefer to find a more convenient way.

@r3c

This comment has been minimized.

Show comment
Hide comment
@r3c

r3c Oct 10, 2016

Owner

Good! I have no better solution for the moment so I'd rather close the issue, but I'm still open to any idea that would allow easier syntax with no configuration parsing code duplication.

Owner

r3c commented Oct 10, 2016

Good! I have no better solution for the moment so I'd rather close the issue, but I'm still open to any idea that would allow easier syntax with no configuration parsing code duplication.

@r3c r3c closed this Oct 10, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment