linz / geostore

Central storage, management and access for important geospatial datasets
MIT License
33 stars 2 forks source link

Create initial Data Lake CD #6

Closed imincik closed 4 years ago

imincik commented 4 years ago

Task

Acceptance Criteria

imincik commented 4 years ago

I just realized that in order to build CD we have to create initial CDK project (cdk init) + requirements file. New ticket for this is needed.

SPlanzer commented 4 years ago

See https://github.com/linz/bde-processor-deployment/tree/master/.github/workflows for example

SPlanzer commented 4 years ago

It appears to me that bde-processor-deployment workflows do not depend on / trigger each other. There is a label used to limit one off their uses.

https://github.com/linz/bde-processor-deployment/blob/d62f8f863e9cd5256e991fd2c090977351c1246c/.github/workflows/test.yml#L18

but nothing to have one rely on the other.

As of the Aug 2020 API update workflows can be triggered via the API (https://developer.github.com/v3/actions/workflow-runs/#get-a-workflow-run). Therefore one workflow can trigger another with a curl command. There is also a GitHub action for this - https://github.com/peter-evans/repository-dispatch

It is however my opinion that this over complicates the CI/CD workflows and we should reply of the GitHub actions needs functionality to manage job dependencies (at-least for now)

imincik commented 4 years ago

It appears to me that bde-processor-deployment workflows do not depend on / trigger each other. There is a label used to limit one off their uses.

Yes, that's right.

imincik commented 4 years ago

As of the Aug 2020 API update workflows can be triggered via the API (https://developer.github.com/v3/actions/workflow-runs/#get-a-workflow-run). Therefore one workflow can trigger another with a curl command. There is also a GitHub action for this - https://github.com/peter-evans/repository-dispatch

It is however my opinion that this over complicates the CI/CD workflows and we should reply of the GitHub actions needs functionality to manage job dependencies (at-least for now)

I am ok with both - sequential workflows or sequential jobs once it's guaranteed that execution of deployment step will happen only when CI step is successfully finished.

imincik commented 4 years ago

@SPlanzer , I would seriously consider using CDK Pipelines library (https://docs.aws.amazon.com/cdk/latest/guide/cdk_pipeline.html)

SPlanzer commented 4 years ago

I have had a good look at CDK Pipelines.

My concern is this adds much more complexity and components than just using CDK/github actions (for example in the way basemaps does) without large benefit.

For example it adds aws Codepipe which seems like a great product but does much (possibly more?) of what GitHub actions already does.

I think if we were to go down the CDK Pipelines path we should select this over GitHub action but this breaks away from the current LI CI/CD pattern and I do not have a good reason to recommend a change in technology stacks.

I am going to therefore take a very similar approach to basemaps - I will implement this tomorrow NZ time.

@palmerj do you have any input on such technology stack decisions concerning CD?

palmerj commented 4 years ago

Hi both. I dont have time to look at this in detail but on the surface feels we should use CDK for CD like basemaps does, and avoid the extra dependency. I'm also not clear reading your comments why this is needed? What's the issue?

SPlanzer commented 4 years ago

I'm also not clear reading your comments why this is needed? What's the issue?

I was an consideration if we should be using the new aws cdk pipelines for CD. It was suggested you may have an opinion on this approach/technology.

The decision is to precede as per basemaps

palmerj commented 4 years ago

I was an consideration if we should be using the new aws cdk pipelines for CD. It was suggested you may have an opinion on this approach/technology.

No opinion, because I haven't used it. But what are functional reasons to consider it?

The decision is to precede as per basemaps

All good.