Peg openpyxl dependency to 2.4.9 to avoid non-backwards compatible api change

[andrewgy8]

As mentioned here and here, openpyxl has changed there api for xlsx. Currently when exporting to xlsx we receive a 500 error.

Addresses #762

[coveralls]

Coverage increased (+0.02%) to 92.501% when pulling 25f17e3 on andrewgy8:master into 9007be3 on django-import-export:master.

[bmihelac]

Hi @andrewgy8 and thanks for filling this pull request. I wonder if we should wait until this issue is fixed in tablib upstream?

[andrewgy8]

Hey @bmihelac!

I would say that would be the best course of action. But it does not seem like we can be sure when tablib will have this fixed/released. As you can see in this issue, there seems to be some worry from the community about the maintenance and governance in general.

I would say the next best course of action, is for ourselves to be proactive (this PR) AND watch closely the progress of the issue in tablib that addresses this error. And it might be a healthy exercise to watch the state of tablib in general.

[lubo]

@andrewgy8 is right, we should stick to versions that are known to work. Although I'd stick to openpyxl 2.4 series, rather than to 2.4.9 in particular. Something like openpyxl>=2.4,<2.5.

[andrewgy8]

@bmihelac I have updated this PR. What do you think? Shall we peg and wait?

[dv321]

This is causing exports to break for me unless I peg the openpyxl version, and since the resolution of the tablib issue seems unclear I'd humbly recommend that this be merged so the library doesn't come broken out of the box, at least until the tablib fix goes through.

Edit: It seems that https://github.com/kennethreitz/tablib/pull/296 already fixes this, so maybe just waiting to peg the version of the next tablib release would be smarter?

[bmihelac]

Sorry for late reply. So what is the best action here? wait for tablib or peg?

[andrewgy8]

@bmihelac No worries. I would suggest merging this. In my opinion its best to be proactive rather than reactive. We can bump the pinned version when underlying libs have this fixed in their releases.

[aparakian]

Any update on this? That would be great to have this merged since it seems to solve the issue quite well.

[andrewgy8]

@aparakian Still in a holding pattern for a member to merge.

@bmihelac Any chance we can merge this soon?

[bmihelac]

Merged, @andrewgy8 thanks for patch!

[aparakian]

Thank you @bmihelac for the reactivity! ⭐️

[andrewgy8]

No worries @bmihelac 👍

I'd like to help out as much as I can as we use this package quite extensively. Ill keep an eye out for other patches I can submit 😉