setup.py: Removed httpretty dependency

[nikicc]

Checking whether httpretty is really required? When looking at code I could not find any imports of httpretty. The file test_smart_open.py uses mock — an other library for mocking. This also explain, why some versions broke tests. So, I suppose, that httpretty is here only due to some legacy reasons and can therefore be removed.

@tmylk can you double check this? I think we can remove httpretty from dependency list and thereby also resolve some issues.

[tmylk]

Thank you for investigating it further.

Httpretty is pinned by moto.

See Travis logs:

Collecting httpretty==0.8.10 (from moto) Downloading httpretty-0.8.10.tar.gz (41kB)

Hopefuly we can get to the bottom of this soon.

[tmylk]

Closing to avoid duplication with #59 and #40

[nikicc]

Did not know this, my bad 🙈

[nikicc]

@tmylk after some more thinking I still believe that we should remove this.

If this was here just for moto, we should remove it from the smart_open setup.py. If moto only works with httpretty==0.8.10 then this should be stated just in moto's setup.py (as it already is) — I do not see the need to restate this here since httpretty is not our dependency. Also, since moto is only needed for running tests on travis and – correct me if I am wrong – we do not need it to run smart_open, removing httpretty from setup.py we would also resolve issue #40. When installing through pip the issue #40 could persist since httpretty will not be among the dependencies any more.

Even if I am wrong, and removing it would not resolve #40 I still believe that restating moto's dependencies is unnecessary and should be removed. What do you think?

[tmylk]

Thanks for your suggestion and investigating it further. You are welcome to remove this requirement in your private fork.
But we cannot do it in the main project because of quality. The test environment has to be exactly the same as when smart_open is used in the wild. If dependencies are different between test and what we ship then our Travis tests become useless.

[nikicc]

If so, why do you then install mock moto & responses in .travis and not in setup.py? They also are different in test environment than in the wild. I am not trying to be smart here, I just don't get it 😄

[nikicc]

Further, if httpretty is only here for moto, then why isn't moto also in setup.py? As I see it, when pip installing, it breaks due to httpretty==0.8.10 which is here just due to moto; however moto itself would never even be installed by pip since it is not a dependency. Do you agree?

Thanks for being patient with me and sorry for bothering you so much, but I really cannot see the reasons behind this.

[tmylk]

Do you agree that if the httpretty pin is dropped the Travis will be different from your LC_ALL=C setup?

[nikicc]

Yes, I agree that it will be different. However, it will only differ in httpretty=0.8.10 which is not the dependency of smart_open. It will only differ in a dependency that is never needed in production! Why do you care about if there is an additional library installed on the user's computer, if it is never imported and never used?

Before reading the rest of my discussion, let me stress that my only intention is to help. To help me fix the issue and to help other who might encounter it. I am not trying to compromise test and stability. I wouldn't be proposing this otherwise. If you convince me that doing this is harmful I will stop and not bring it up again.

Let me try to convince you one more time. Here are my reasons:

• If you want the environment in the wild to be the same as on Travis (which we all strive for), why is it ok to exclude mock moto responses from setup.py?
• Since mock moto responses are not in setup.py they will never be installed when installing smart_open through pip. There will always be at least this difference between Travis and the wild. If so, why do we care whether httpretty is on the system and which version? It will never be used anyway. When pip installing you cannot run test, since mock moto responses are missing, so why do we care that there is a right version of httpretty? What difference does it make?
• If httpretty is only needed by moto and not by smart_open, why is then moto installed only for Travis (through .travis) and httpretty through setup.py? Shouldn't they both be either in .travis or in setup.py?
• Imagine that moto fixes this tomorrow and decides that it needs a 0.8.14 version of httpretty. What would happen? They would issue a new release, the next time your Travis run it would take this new version of moto and break due to a version conflict. Then you would have to fix it here and also make a new release. That is an unnecessary duplication. My reasoning is the following. We should not even think about httpretty. All we really need is a working version of moto. Do you agree? If so, we should let the authors of moto decide which versions they want — as they are currently doing by pinning the version. If the current version of moto is working for us, what do we gain by pining httpretty? On Travis we get 0.8.10 either way, with pip it currently breaks if LC_ALL=C, but could work if the pin is removed. Further, if we were to pin anything, which I oppose if not really required, the only thing we could pin is the moto itself, not its dependencies. We require moto, we do not require httpretty, therefore only moto should be among our dependencies.
• Look at Remove requests version pin. Also support Python 3.5 in setup.py #67. There you removed requests from setup.py. Why can't we do the same for httpretty? In what sense would we compromise test with removing the pin? They would run on exactly the same environment and the pass/fail decisions would not be affected. All we would be changing is what gets installed when we use pip.

Hope that you understand my concerns. Please, let me know what you think about this.

[tmylk]

Hey,

It is good to keep talking about it as it makes the logic clearly.

On Travis we get 0.8.10 either way, with pip it currently breaks if LC_ALL=C, but could work if the pin is removed.

What makes you think that it 'could' work if the pin is removed? Automated tests don't run in that setting. I have no idea if it works or not.

Do you suggest we run some manual tests here?

[kernc]

The same @piskvorky's argument applies: If there are any requirements pertaining to tests only (e.g. moto, mock and responses), you should set them in setup.py's tests_require (can't blame you for not being aware of this; setuptools suck) and then only run python setup.py test on Travis. Only specify in install_requires the absolute necessities required for your library to functions. This currently makes smart_open and thus gensim unbuildable in confined environments that set LC_ALL=C (i.e. environments that want default English but don't know exactly what other locales are available on the system).

What makes you think that it 'could' work if the pin is removed?

Was the pull request green before it was closed?

[nikicc]

What makes you think that it 'could' work if the pin is removed?

If we remove the httpretty from our dependencies, pip will never even try to install it. Hence, I believe it should work.

Automated tests don't run in that setting. I have no idea if it works or not.

Yes, they do! Look at this PR. After I removed httpretty from the dependencies all the tests run just fine.

The reason they run just fine is that nothing whatsoever changes for Travis. The environment there will be exactly the same. Look at the current Travis log at line 130. httpretty==0.8.10 is installed by the command pip install mock moto responses, and not from our setup.py. In a sense, it's redundant to have it in our setup.py since it is moto's dependecy. So if we remove it from out setup.py httpretty==0.8.10 will still be installed on Travis, since moto's setup.py dictates so. However, it will not be installed when pip install smart_open, which is ok, since smart_open only needs it for testing and not in production.

Do you suggest we run some manual tests here?

No. We should make with work with Travis — as a matter of fact, it already works there 😄

[nikicc]

@tmylk any thoughts on this?

[tmylk]

Could you suggest a way to automatically test a setup that allows for LC_ALL=C?

Moto currently doesn't work with it because of httpretty.

[nikicc]

I think we should wait for Moto to fix it's problems first, before adding the test. Currently, we cannot set LC_ALL=C for whole Travis environment since Moto does not work with it. As of now, we could only do it dirty like LC_ALL=C python setup.py install. However, when Moto fixes its problems, we should set it globally like in setuptools.

Either way, I believe we should remove httpretty from our dependencies ASAP. Currently, we do not have this test and it does not work with LC_ALL. If we remove httpretty we still won't have the test but it should work with LC_ALL. The fact that we cannot add the test (due to moto's problems) should not be a reason not to apply the fix. Though, I agree that the test should be added once moto fixes it's problems.

[nikicc]

@tmylk just a reminder, could we consider removing httpretty?

[tmylk]

@nikicc Yep, thanks for the perseverance. Let's merge. Please let me know if it works for you. You are our only test in this case. :)

[nikicc]

@tmylk: Thanks! I'm very happy to see it merged 😉

I build a package with python setup.py sdist and installed it with LC_ALL=C and it worked!