Skip to content

Initial draft of Python PIP builder design doc - #5

Merged
sanathkr merged 2 commits into
developfrom
python-pip-design
Nov 9, 2018
Merged

Initial draft of Python PIP builder design doc#5
sanathkr merged 2 commits into
developfrom
python-pip-design

Conversation

@stealthycoin

Copy link
Copy Markdown
Contributor

Initial draft of Python PIP builder design doc. Let me know what you think.

@sanathkr sanathkr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

WOW! I never realized you had to do so much shenanigans to build Python for Lambda reliably!

Super excited to have such a robust, well-designed and well-documented library as part of this project 🔥

I have some minor comments about the interface.

built for it. It is assumed that this customer code is correct and
would function if it had all of its dependencies installed alongside
it. It is assumed that the dependencies are specified in a top level
``requirements.txt`` file. This file can be removed before deploying

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would assume that this method is passed a path to requirements.txt. SAM CLI will ensure the file is at top-level within the source folder but this should not be an assumption that low level build actions must make.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok I can make this more in line with the params outlined in your other PR. I will still assume I have an adapter between them to make this low level builder more extractable and specific to python.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes sense

in this directory.

:type config: :class:`lambda_builders.actions.python_pip.config`
:param config: A configuration object which contains additional config

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would call this out as optional, and not implemented, for now. This requirement exists for several build actions. So I want to standardize this as Optimizations that can turned on/off by customers - #3

@stealthycoin stealthycoin Nov 9, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah yes I meant to write that, my bad. Thats what I meant by =None in the signature :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah I missed it ;)

at the end and not interrupt the rest of the dependency building.
2) Return None or a list of aggregated error objects that can be inspected by
the caller.
3) Use the UI object. The builder as it is written right now has a notion of a

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like the idea of a UI object. This allows us to abstract functionality like "write with colors" etc within the object. We could also redirect the data to a file transparently. Feel free to make assumptions about this object for now. We can implement a real UI object separately.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Cool. For now Whatever wrapper around this low level builder can create and inject it.

@sanathkr

sanathkr commented Nov 8, 2018

Copy link
Copy Markdown
Contributor

Checkout https://github.com/sanathkr/aws-lambda-builders/blob/builder-interfaces/lambda_builders/actions/python_pip/entrypoint.py to get a rough idea of the parameters passed to the python_pip action

@sanathkr

sanathkr commented Nov 9, 2018

Copy link
Copy Markdown
Contributor

Let me know once you have pushed a new draft. I will approve and merge

@sanathkr sanathkr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@sanathkr
sanathkr merged commit 3b70beb into develop Nov 9, 2018
@sanathkr

Copy link
Copy Markdown
Contributor

Accidentally deleted this file from the repo after creating the new directory structure. I will add this to my other open PR

sanathkr added a commit to sanathkr/aws-lambda-builders that referenced this pull request Nov 11, 2018
Original file got accidentally deleted during a refactor.
sanathkr added a commit that referenced this pull request Nov 12, 2018
Original file got accidentally deleted during a refactor.
sanathkr added a commit that referenced this pull request Nov 19, 2018
Original file got accidentally deleted during a refactor.
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.

3 participants