Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
  • 13:58
    codecov[bot] commented #1590
  • 13:55
    codecov[bot] commented #1590
  • 13:55
    codecov[bot] commented #1590
  • 13:48
    codecov[bot] commented #1590
  • 13:48
    codecov[bot] commented #1590
  • 13:48
    vytas7 synchronize #1590
  • 13:13
  • 13:08
    codecov[bot] commented #1604
  • 13:08
    codecov[bot] commented #1604
  • 13:08
    codecov[bot] commented #1604
  • 12:57
    codecov[bot] commented #1590
  • 12:57
    codecov[bot] commented #1590
  • 12:57
    vytas7 synchronize #1590
  • 12:57
    codecov[bot] commented #1604
  • 12:57
    codecov[bot] commented #1604
  • 12:57
    vytas7 synchronize #1604
  • 12:56

    vytas7 on master

    doc(changelog): add supported p… (compare)

  • 12:56
    vytas7 closed #1598
  • 12:54
    vytas7 commented #1604
  • 12:43
    vytas7 commented #1604
Vytautas Liuolia
@vytas7
@mikeyusko That's true, seems like a leftover. You can create a follow-up PR.
It seems we happened to miss @nZac 's comment on falconry/falcon#1592, and forgot to add a Towncrier newsfragment. So that fragment could also go into the follow-up PR.
OTOH re api_helpers.py, one could argue those helpers are private helpers supporting the Falcon API interface, not the API class, so probably not a big deal either way for me. But it is probably a good idea to rename for consistency provided we have renamed everything else.
Mykhailo Yusko
@mikeyusko

and forgot to add a Towncrier newsfragment.

I think that I added it because I pushed the latest changes from the master branch into my branch.

app = falcon.API(middleware=[
    AuthMiddleware(),
    RequireJSON(),
    JSONTranslator(),
])
also, it should be changes as well, in a README section. falcon.API -> falcon.App
Vytautas Liuolia
@vytas7
FWIW, @mikeyusko I see no new news fragments in the master branch that would have been added recently: https://github.com/falconry/falcon/tree/master/docs/_newsfragments .
Mykhailo Yusko
@mikeyusko
oh, sorry for the misunderstanding. @nZac meant to create file and add it manually, am I right?
Nick Zaccardi
@nZac
The news fragements should be a part of the PR, in my estimation... open to a better workflow if there is one. (That is what you are asking, yes?)
Mykhailo Yusko
@mikeyusko
yes.
Nick Zaccardi
@nZac
@vytas7 I was looking at your multi-part stuff last night and within 10 mins I was deep in the Cython docs :sweat_smile:
Vytautas Liuolia
@vytas7
So was I when developing :slight_smile: I have another pure-Cython PR out there for optimizing URI query parsing as well :slight_smile:
Vytautas Liuolia
@vytas7
Cython does get a bit rough on the edges if you delve deeper into it... With some more love and polish it could become a great language.
Mykhailo Yusko
@mikeyusko

@vytas7 @nZac I'll create a follow-up PR for this one falconry/falcon#1592

  1. Rename api_helpers -> app_helpers to be consistent
  2. Add newsfragment (could you provide a bit more info on how to do it? because I can't understand for which goals it needed)
  3. Fix falcon.API in the README file.

Thanks

Vytautas Liuolia
@vytas7
@mikeyusko Sounds like a plan. Could you also take a look at the "look" tutorial tests? https://travis-ci.org/falconry/falcon/jobs/622120513
At least the decorator is working as expected :wink:
Mykhailo Yusko
@mikeyusko
@vytas7 Yup, will do
coverage is 75%, we need 100% :smiling_imp:
Mykhailo Yusko
@mikeyusko
@vytas7 ok, I have looked at the "look" tests, and I don't have any ideas right now, how to improve the coverage.
Vytautas Liuolia
@vytas7
@mikeyusko Sorry for the confusion if that was my fault. We don't even track coverage for "look" AFAICT. Where do you get that 75% from? :confused:
What I meant was that running "look" tests seemed to emit the API deprecation warning, as if these tests hadn't been upgraded to use App yet.
Vytautas Liuolia
@vytas7
examples/look/look/app.py
10: api = falcon.API()
Vytautas Liuolia
@vytas7
examples/look/tests/test_app.py ... [ 75%] :arrow_left: @mikeyusko if you mean that 75%, it is, to the best of my knowledge, merely a pytest progress indicator (3/4 tests complete at that point).
Mykhailo Yusko
@mikeyusko

@vytas7 and yeah, thank you for your support, it’s really helpful, and I day by day much more understands falcon’s flow, principles, and rules.

And as a result - contribute to Falcon will be a bit easier

:thumbsup:
Vytautas Liuolia
@vytas7
@kgriffs Thanks for your fantastic review of the Cython query string parser stuff. I'll try to address the comments this weekend.
If we are to accept some pure Cython stuff into Falcon, I'm also experimenting with an IBM Z (s390x) pipeline in Travis, as it seems to be Big Endian architecture.
Woohoo multipart Cython parts passed on Big Endian :tada:
Kurt Griffiths
@kgriffs
sweet
Vytautas Liuolia
@vytas7
I'm a bit divided on your booleans suggestion @kgriffs
I will probably need opinions :)
Kurt Griffiths
@kgriffs
tbh, it is mostly just about personal preference. I don't think a couple of extra operations is going to be noticeable in terms of CPU cycles.
Vytautas Liuolia
@vytas7
You are in general right. However, if estimating that having a value is more common than not, that's what I'm interestingly getting
>>> import timeit
>>> k = 'key'; v = 'value'; keep_blank = True
>>> timeit.timeit(lambda: not (v or (keep_blank and k)), number=int(1e8))
5.702389151003445
>>> timeit.timeit(lambda: not (v or (keep_blank and k)), number=int(1e8))
5.684955962999084
>>> timeit.timeit(lambda: not (v or (keep_blank and k)), number=int(1e8))
5.685243817999435
>>> timeit.timeit(lambda: not v and (not keep_blank or not k), number=int(1e8))
5.500417819006543
>>> timeit.timeit(lambda: not v and (not keep_blank or not k), number=int(1e8))
5.490288328001043
>>> timeit.timeit(lambda: not v and (not keep_blank or not k), number=int(1e8))
5.471835395997914
Kurt Griffiths
@kgriffs
oic
Vytautas Liuolia
@vytas7
I.e. my version is actually slightly outperforming anything else if v reduces to True.
Kurt Griffiths
@kgriffs
that makes sense.
Vytautas Liuolia
@vytas7
These boolean ops are pretty speedy even in Python though :slight_smile:
This was CPython 3.8.0, FWIW.
Kurt Griffiths
@kgriffs
Keeping track of the parsing state in my head was a bit tricky but I whiteboarded everything out and didn't see any flaws in your logic. But hopefully I didn't miss something. :wink:
Vytautas Liuolia
@vytas7
Aye, the whole story is somewhat funny because I'm not that much of a performance junkie as some other Falcon maintainers :slight_smile: But once you dig deep into the problem, it becomes so cool to shave off that nanosecond in some obscure Cython optimization :slight_smile:
Vytautas Liuolia
@vytas7
@kgriffs do you think it is worth keeping a Big-endian CI gate for us? Anyway, I added it to the PR just as a test pilot for now. We can remove it later if we feel confident.
Kurt Griffiths
@kgriffs
Better safe than sorry as they say.
Vytautas Liuolia
@vytas7
Let's see if it passes first tho...
Kurt Griffiths
@kgriffs
lol
Vytautas Liuolia
@vytas7
It passed for multipart, but I haven't tested it for URI parsing in question yet.
E: Unable to locate package libunwind-dev
The command "sudo apt-get install -y libunwind-dev" failed and exited with 100 during .
Oh, failed with something mundane :grimacing: I think it was me who added this libunwind-dev for our benchmark suite to work on 16.04, I could try to make it conditional
Vytautas Liuolia
@vytas7
I was hoping arm64 would be Big-endian, but it was not on Travis. Modern ARM chips can use either endianness (allows switching).
Vytautas Liuolia
@vytas7
Falcon is now "certified" on IBM Z (s390x) :slight_smile: : https://travis-ci.org/falconry/falcon/jobs/624750430