-
-
Notifications
You must be signed in to change notification settings - Fork 337
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.
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
Fix errors in manifest file #34
Conversation
Hi, Weird. Just tried myself and works fine without your changes. What versions of pip + python are you using and what OS? Here's mine on OSX Mavericks:
|
Windows 8.1
|
…cle, to reduce Silk's overhead. Fixed redefinition of silk_queries in DataCollector.
…ved in each request cycle
…types whose compilers define their own execute_sql methods
…nique constraint "silk_response_request_id_key"
Hey @joaofrancese, FYI I just had to revert this. All the tests are failing and things have changed too much for me to fix them quickly (and I'm really short on time for the next month or so). Two major changes have broken the tests, the change from I seem to remember that the reason I didn't use If you're willing to incorporate your changes into the unit tests and travis build I can give you commit rights to repository so that you don't have to wait for me to approve. If Also, if you look at the Cheers! |
I implemented set_primary_keys in bulk_create() in my fork of django-mssql, using an SQL Server-specific solution, because the inability to use bulk_create in such situations was severely hampering my project's speed. That's why I didn't make a pull request of my changes. Though, the way I used it in silk should be backwards-compatible, because I check for a flag I created in DatabaseFeatures, and fall back to saving each object individually in case the flag doesn't exist. It's just a guess, but the test may be failing because I moved the updating of request.num_sql_queries from save() / bulk_create() to DataCollector.finalise(). The previous implementation in bulk_create wasn't working, because it was updating the Request object directly in the database using update() with F expressions, but the in-memory request object was saved afterwards, negating these changes. I'd have to fix it anyway, thought it would just be easier to have the code in only one place, so I put it in finalise() and removed the manager. It would create inconsistencies if SQLQuery objects are saved outside of this method, but I looked around it doesn't seem to be the case. Do you think it's okay to leave it this way? I'll review the tests to fix either the code or the tests. |
Ah right understood! It seems that when you've made a pull request and then commit further changes after the fact, github pushes those commits into the pull request too! So when I merged this in I got everything, not just the manifest changes... Not sure why that's the behaviour they chose. Wrt. the flag checking, cool, somehow I missed that addition, if the tests pass and the travis build passes then we can know for sure that the backwards compatibility works. Wrt. Awesome, much appreciate that. |
I am getting the following error when installing with Pip from PyPI or from source (with -e git+https://...)
I was able to install with Pip from source from my fork after this change to the manifest file.