Skip to content
This repository was archived by the owner on Dec 9, 2024. It is now read-only.

Conversation

@ddoowa
Copy link
Contributor

@ddoowa ddoowa commented Nov 12, 2019

Added Functionality to define volumes to attach the function pods to, also refactored the process to define secrets to re-use the same underlying function.

example volume definitions:

functions:
  my-function:
    handler: handler.myFunction
    volumes:
      - name: my-volume
        persistentVolumeClaim:
            claimName: my-volume
    volumeMounts:
      - mountPath: /foo/bar
         name: my-volume

Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @ddoowa!

I have a few suggestions, apart from that I think it's better to diverge as less as possible from the format that a pod expect for volumes. In particular I would suggest:

functions:
  my-function:
    handler: handler.myFunction
    volumes:
      - name: my-volume
        persistentVolumeClaim:
          claimName: my-volume
    volumeMounts:
      - mountPath: /foo/bar
        name: my-volume

That way it's easier for people to know the format and it's easier to use in the code (you don't need to parse and translate the information from the function spec).

@ddoowa
Copy link
Contributor Author

ddoowa commented Nov 13, 2019

Thanks for the PR @ddoowa!

I have a few suggestions, apart from that I think it's better to diverge as less as possible from the format that a pod expect for volumes. In particular I would suggest:

functions:
  my-function:
    handler: handler.myFunction
    volumes:
      - name: my-volume
        persistentVolumeClaim:
          claimName: my-volume
    volumeMounts:
      - mountPath: /foo/bar
        name: my-volume

That way it's easier for people to know the format and it's easier to use in the code (you don't need to parse and translate the information from the function spec).

@andresmgot That sounds even better, not sure why i didn't think of it, but i have now made the necessary changes to support this format, and addressed other comments, looking forward to the merge.

Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

Great, thanks for the changes!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants