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.

Sign up for GitHub

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

feat: Error on --use-container for dotnet builds #1096

Merged
merged 7 commits into from
Apr 8, 2019
Merged

feat: Error on --use-container for dotnet builds #1096

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bd98c09
feat: Bypass containers for dotnet builds
sanathkr Apr 2, 2019
ce2efae
Merge branch 'develop' into dotnet-bypass-container
sanathkr Apr 5, 2019
022a9eb
Fail fast if container build is not supported
sanathkr Apr 5, 2019
a117338
Better error message
sanathkr Apr 5, 2019
376ae11
Integration test for dotnet container build case
sanathkr Apr 5, 2019
50907bb
Deriving a key from workflow config because namedtuples can't be hase…
sanathkr Apr 6, 2019
7b82db8
stringify before creating the key
sanathkr Apr 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion samcli/lib/build/workflow_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def supports_build_in_container(config):
"""

def _key(c):
return c.language + c.dependency_manager + str(c.application_framework)
return str(c.language) + str(c.dependency_manager) + str(c.application_framework)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn’t this be the hash function on the object? It’s much cleaner that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The whole point of namedtuple is so you automatically get a sane hash function. I don't want to break that abstraction by defining a custom has that takes only a handful of properties. May be I am wrong here, so let me know if a custom hash is right way of doing things..


# This information could have beeen bundled inside the Workflow Config object. But we this way because
# ultimately the workflow's implementation dictates whether it can run within a container or not.
Expand Down