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

Support custom generators #57

Closed

Conversation

michalsnik
Copy link

@michalsnik michalsnik commented Mar 31, 2017

What Changed & Why

This PR resolves #23, by adding ability to use custom generators on top of choosen one. User is able to override outcome of the selected generator or write completely unique one if needed.

Related issues

#23

PR Checklist

  • Add tests
  • Add documentation
  • Prefix documentation-only commits with [DOC]

People

@Bockit
@jrowlingson
@lukemelia

@achambers
Copy link
Member

Thanks @michalsnik. I'll have a chat with @lukemelia and @ghedamat about this as there are a few things in this area that we need to sort out. I'm not personally convinced this is the correct approach to what you're trying to do.

That aside, the generator property function you have here doesn't conform to how every other config prop function works which can make thing a confusing to consumers.

It is generally expected that config property functions will be called with the context object passed in as opposed to your data object. This is done as it's encouraged that plugin authors use this.readConfig to access config props.

I wouldn't be comfortable using the approach you have as the config prop in question works differently to the conventions we've created.

I'll get back to you after I talk through a few other things with the team.

Thanks for your contributions to making the eco system a better place :)

@achambers
Copy link
Member

@michalsnik Thinking more about this, can you please give me some more context around your use case? What is the custom hash you're hoping to use.

Essentially what you have done in this PR is to allow a user to provide a function that returns a revision key. This is exactly how ember-cli-deploy config prop functions work and so, you can actually do this without any change required to this plugin. Instead of specifying the function in the revision data plugin, you simply specify the revisionKey config property of the redis (or whatever plugin needs it) as a function that returns the revision key value.

Does this make sense? Would it cater for your needs?

@michalsnik
Copy link
Author

michalsnik commented Apr 3, 2017

Hey, what I needed was to get revisionKey like this: 4cae3e0+2017-03-31T13:25:59.003Z, so commit hash + timestamp.

I didn't want to get commit hash on my own, because you already provide this in a nice manner. So I thought it would be nice if I could create custom generator that takes one parameter, which is an object returned by your Data Type generator and do with it whatever I want. In my case you generate this kind of object:

{
  revisionKey: '4cae3e0',
  timestamp: '2017-03-31T13:25:59.003Z'
}

So I can do something like this:

type: 'git-commit',
generator(data) {
  return Object.assign({}, data, {
    revisionKey: `${data.revisionKey}+${data.timestamp}`
  });
}

@achambers
Copy link
Member

@michalsnik Yep, so you can do exactly this in the way I suggested above. Assuming you are using the redis plugin (but this will apply for any plugin whereby you need the revision key), you can do this:

ENV.redis = {
  revisionKey: function(context) {
    var data = context.revisionData;
    return `${data.revisionKey}+${data.timestamp}`;
  }
};

@michalsnik
Copy link
Author

I'll check this out. Thanks!

@ultimatemonty
Copy link

@achambers I have a use case I think this PR would perfectly address.

I'm currently working on updating our deployment strategy and one of things I'd like to do is use a revisionKey that matches the build number from our CI system. So instead of a version, git-sha, or some combination there-of it would be build302. The assets would stay fingerprinted as-is but I'm planning to use the json-config plugin to easily reference the assets for a particular build. I'd like to be able to identify the index.json associated with a particular build by using custom revisionKey so I end up with something like index.json:build302 which then contains the assets for that specific build.

Thanks for all the work the team has put into this ecosystem - it's fantastic!

@achambers
Copy link
Member

Hi @ultimatemonty. Would I be accurate in saying your deploy will be happening in CI and therefore the CI build number will be available as an environment variable, such as TRAVIS_BUILD_NUMBER.

@ultimatemonty
Copy link

@achambers you are correct that the build number is available in CI as an environment variable but we do not ship to production directly from CI. We will call ember deploy in CI but just to build the assets and generate the index.json file.

@achambers
Copy link
Member

@ultimatemonty said:

you are correct that the build number is available in CI as an environment variable but we do not ship to production directly from CI. We will call ember deploy in CI but just to build the assets and generate the index.json file.

Can you please elaborate a little more on what the flow looks like and from which systems you intend to call the deploy commands from?

What do you mean by:

We will call ember deploy in CI but just to build the assets and generate the index.json file.

I'm not clear on what you're intending to happen after this. Once the assets are build on CI, what are you intending to happen to them? When will they be shipped somewhere?

Sorry for the quesitons. I'm just trying to get the context from where you are asking your questions as I am not convinced custom data generators is the way forward for this plugin and it isn't inline with the vision I have for the plugin. And, in my experience, there is usually always an alternative to this approach. The more info you give me, the better I can help you out.

@ultimatemonty
Copy link

@achambers no worries!

So our basic process is we run tests and build assets in CI. Those assets are shipped from CI to an artifact server where they are stored with some metadata about that particular build (pass/fail for tests, build number, git sha, etc...)

We ship builds manually instead of using continuous deployment.When we ship a build we have processes that will request the proper build artifacts from the artifact server and pull them down to the application servers.

Let me know if you need more info!

@achambers
Copy link
Member

@ultimatemonty Sorry for the back and forth.

Ok, so essentially you will only be running ember deploy (as opposed to ember deploy:activate as well) and this will indeed happen from your CI server.

If this is the case then which plugin are you using to ship the assets to the artifact server? Do you have your own custom plugin or are you using an existing ssh plugin or something?

@ultimatemonty
Copy link

@achambers again no worries - we're all learning something in this process :)

We have some custom scripts that perform the asset and metadata uploading that are called from our CI environment. The steps are build the assets, ship assets and known metadata to artifact server, run test suite, update metadata if test suite passes. So we always ship the assets and metadata and then update the metadata if tests pass

@achambers
Copy link
Member

@ultimatemonty Ok, so where exactly does ember-cli-deploy fit in to that then? I must be missing something here?

@ultimatemonty
Copy link

@achambers it mostly centers around the json-config plugin. Having the assets for a build listed out in that json file will make it much easier for us to reference the correct assets for a build. Rather than roll my own script to handle that I just use the existing ember-cli-deploy ecosystem without actually deploying anything - just building.

@achambers
Copy link
Member

@ultimatemonty Ok, I'm starting to form a mental model around where your thinking is at. The final thing I need you to clarify is when you mentioned you're after something like index.json:build302. What is that exactly? Is that a file name? Or is that revision key you're after?

I'm just not clear on where you're planning on using the revision key. The reason being, if I'm correct, what you're describing is really an ember-cli-deploy pipeline that consists of the build plugin and the Jain-config plugin. And all json-config does is take what is in index.html and put it into an index.json file.

In this scenario, the revision-data plugin would never actually be used. This plugin is used to generate a unique revision key which will essentially be used as the key for which the index.{html,json} file will be uploaded in, say, redis. But you aren't planning on doing that so I'm a little unclear on your vision for the revision key in the first place.

@ultimatemonty
Copy link

@achambers I hadn't even thought about where in the pipeline the revision key is generated. You are absolutely correct that we would never get to that stage since we will be shipping the assets outside of the ember-cli-deploy pipeline.

Thank you for taking the time to work through this with me even if it ultimately resulted in a no-op!

@achambers
Copy link
Member

No probs @ultimatemonty . One final thing I will ask though is, have you considered using ember-cli-deploy for the building and shipping of your assets to the artifact server, instead of your home rolled scripts? Even the "processes that will request the proper build artifacts from the artifact server and pull them down to the application servers". This is exactly the sort of thing ember-cli-deploy is built for.

Worth considering. And myself and the rest core team are more than happy to help you along the way.

Just holler if you need anything else :)

@achambers
Copy link
Member

Closing this PR now as this is not the direction that we have for this plugin

@achambers achambers closed this Apr 29, 2017
@ultimatemonty
Copy link

@achambers we have a ton of projects that use the same basic pipeline described above to ship to production. We want to keep ours in line with that same pipeline. Otherwise I would absolutely be using ember-cli-deploy for the full deployment.

Thanks again!

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.

Feature: Ability to define a custom data generator
3 participants