Skip to content

Extract ASGI scope creation into function#162

Merged
jordaneremieff merged 2 commits intoKludex:mainfrom
ediskandarov-cur:extract-asgi-scope-creation-into-func
Feb 28, 2021
Merged

Extract ASGI scope creation into function#162
jordaneremieff merged 2 commits intoKludex:mainfrom
ediskandarov-cur:extract-asgi-scope-creation-into-func

Conversation

@ediskandarov-cur
Copy link
Copy Markdown
Contributor

  • Simplify magnum call function
  • extract event processing into dedicated functions

This is a required abstraction to create non-AWS event handlers.

@jordaneremieff
Copy link
Copy Markdown
Collaborator

Thanks for the PR @emcpow2. There is an issue here related to custom scope resolution that should also be considered when making a change like this, and I think if we decide to go with non-AWS event handling that will take some additional design consideration later as well.

I'm going to close this because there is some investigation to do before we start extracting logic from the main adapter method.

@ediskandarov-cur
Copy link
Copy Markdown
Contributor Author

Thit PR does not introduce any complexity regarding the path resolution issue.

Instead, the code looks cleaner and easier to test and to introduce new changes.

Please consider reopening the PR.

@jordaneremieff
Copy link
Copy Markdown
Collaborator

jordaneremieff commented Feb 26, 2021

@emcpow2 I don't disagree with your approach generally, and I've actually thought we should do something like this for awhile now. However, I've held off on these kind of changes because once we introduce new public API then users will start relying on it in ways that conflict with other things we might want to do. This is the same reason I left the linked issue open because the design needs careful consideration.

@jordaneremieff
Copy link
Copy Markdown
Collaborator

jordaneremieff commented Feb 26, 2021

@emcpow2 I've taken the time to think about this some more - let's go ahead with it. I'll reopen and do a small review of some adjustments, but otherwise this should be fine to do now. Thanks.

Copy link
Copy Markdown
Collaborator

@jordaneremieff jordaneremieff left a comment

Choose a reason for hiding this comment

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

Just a few small things then I think this will be good to merge. The most significant change request is to remove the separate method for handling the initial body (we can look at this as a separate issue later).

Comment thread mangum/adapter.py Outdated
Comment thread mangum/adapter.py Outdated
Comment thread mangum/adapter.py Outdated
@ediskandarov-cur ediskandarov-cur force-pushed the extract-asgi-scope-creation-into-func branch from b0c8bc4 to 16371fa Compare February 27, 2021 09:29
extract scope and request body creation into a dedicated functions
@ediskandarov-cur ediskandarov-cur force-pushed the extract-asgi-scope-creation-into-func branch 2 times, most recently from a5b55de to 191b478 Compare February 27, 2021 09:40
@ediskandarov-cur ediskandarov-cur force-pushed the extract-asgi-scope-creation-into-func branch from 191b478 to 5c77151 Compare February 27, 2021 09:43
@ediskandarov-cur ediskandarov-cur changed the title Extract ASGI scope creation into funcions Extract ASGI scope creation into function Feb 27, 2021
@ediskandarov-cur
Copy link
Copy Markdown
Contributor Author

thanks. Done

@jordaneremieff
Copy link
Copy Markdown
Collaborator

Looks good, thanks!

@jordaneremieff jordaneremieff merged commit 18574b8 into Kludex:main Feb 28, 2021
four43 pushed a commit to four43/mangum that referenced this pull request Mar 27, 2021
* 🏗 simplify magnum call function

extract scope and request body creation into a dedicated functions

* 📝 add documentation to create scope function
four43 pushed a commit to four43/mangum that referenced this pull request Aug 20, 2021
* 🏗 simplify magnum call function

extract scope and request body creation into a dedicated functions

* 📝 add documentation to create scope function
khamaileon pushed a commit to khamaileon/mangum that referenced this pull request Jan 13, 2024
* 🏗 simplify magnum call function

extract scope and request body creation into a dedicated functions

* 📝 add documentation to create scope function
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.

2 participants