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

FTPS Support #41

Open
Lutzifer opened this issue Jul 2, 2017 · 4 comments
Open

FTPS Support #41

Lutzifer opened this issue Jul 2, 2017 · 4 comments

Comments

@Lutzifer
Copy link
Contributor

Lutzifer commented Jul 2, 2017

I have begun implementing support for using ftps instead of aws.
The branch can be found here: https://github.com/Lutzifer/carthage_cache/tree/ftps_support
It is not quite ready yet for a PR, as host, username and password are currently set via env variables.

So I just wanted to reach out if you are interested in this kind feature, before putting more effort in the config stuff.

@guidomb
Copy link

guidomb commented Jul 4, 2017

Hi! Thanks for reaching out. I don't use ftps and I'm a little bit reluctant to add features I wouldn't use. My main concern being that I won't be able to maintain it. I'll have to take a look at the final PR but judging by the code in your fork I see that you added double-bag-ftps. I'm not sure how stable / maintained is that dependency. I wouldn't want to be in the situation of having to maintain that dependency as well, although is quite small and single purpose.

It's hard for me to judge if this is a feature carthage_cache users would want. I'll have to think a little bit more about this. I'll have an answer by next week.

If I decide to accept this feature, Would you be willing to be a maintainer?

Thanks again for raising this issue and I hope you can understand my hesitation.

@Lutzifer
Copy link
Contributor Author

Lutzifer commented Jul 5, 2017

Sure, I can understand the hesitation.

Seems like ruby 2.4 has support for ftps out of the box, i will look into that.
Perhaps I can use your library as the base for a fastlane plugin that manages the carthage cache via ftps, which would keep the code decoupled from this repo and adds the functionality and the fastlane plugin I want to have in one single piece of code

@guidomb
Copy link

guidomb commented Jul 5, 2017

That sounds like a very goods option. Let me know if you want to move forward with that option because made we can modify carthage_cache to make it easier for such extension to be developed.

@Lutzifer
Copy link
Contributor Author

Lutzifer commented Jul 9, 2017

This was less work then expected (due to the architecture you used).

A first version, which adds actions for AWS and ftps via fastlane actions can be found here:

https://github.com/num42/fastlane-plugin-carthage_cache_ftps

It is a bit hacky how I create the application though:

When I use this initializer:

  def initialize(project_path, verbose, config, repository: AWSRepository, terminal: Terminal, swift_version_resolver: SwiftVersionResolver)
      @terminal = terminal.new(verbose)
      @archiver = Archiver.new
      @config = Configurator.new(@terminal, project_path, config).config
      clazz = @config.read_only? ? HTTPRepository : repository
      @repository = clazz.new(@config.bucket_name, @config.hash_object[:aws_s3_client_options])
      @project = Project.new(project_path, CACHE_DIR_NAME, @terminal, @config.tmpdir, swift_version_resolver.new)
    end

I have to reuse the structure for AWS to transport my values:

        host = params.values[:host]
        remote_subfolder = params.values[:subfolder]
        username = params.values[:username]
        password = params.values[:password] || PasswordHelper.new.password(host, username)

        config = {
                  bucket_name: remote_subfolder,
                  aws_s3_client_options: {
                    region: host,
                    aws_access_key_id: username,
                    secret_access_key: password,
                    access_key_id: "bla"
                    }
                  }

        local_path = params.values[:local_path]
        application = CarthageCache::Application.new(local_path, true, config, repository: FTPRepository)

If we could change the initializer to simply pass through the config object to the clazz.new call if the repository param is not AWSRepository this would be a lot easier to understand on my side (and no change in behaviour in the original code).

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

No branches or pull requests

2 participants