Skip to content
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

Thumbor 7 #103

Merged
merged 86 commits into from May 6, 2023
Merged

Thumbor 7 #103

merged 86 commits into from May 6, 2023

Conversation

gingerlime
Copy link
Contributor

@gingerlime gingerlime commented Jan 8, 2022

TODO

@gingerlime
Copy link
Contributor Author

@heynemann let's continue the work here? :)

I created a new thumbor:7.0.0 image on docker hub... I tried it and I'm getting this error however:

thumbor_1      | ---> Starting thumbor with 4 processes...
thumbor_1      | Traceback (most recent call last):
thumbor_1      |   File "/usr/local/bin/thumbor", line 8, in <module>
thumbor_1      |     sys.exit(main())
thumbor_1      |   File "/usr/local/lib/python3.8/site-packages/thumbor/server.py", line 142, in main
thumbor_1      |     importer = get_importer(config)
thumbor_1      |   File "/usr/local/lib/python3.8/site-packages/thumbor/server.py", line 61, in get_importer
thumbor_1      |     importer.import_modules()
thumbor_1      |   File "/usr/local/lib/python3.8/site-packages/thumbor/importer.py", line 83, in import_modules
thumbor_1      |     self.import_item("LOADER")
thumbor_1      |   File "/usr/local/lib/python3.8/site-packages/thumbor/importer.py", line 144, in import_item
thumbor_1      |     module = self.import_class(conf_value, get_module=True)
thumbor_1      |   File "/usr/local/lib/python3.8/site-packages/thumbor/importer.py", line 56, in import_class
thumbor_1      |     kls = import_class(name, get_module)
thumbor_1      |   File "/usr/local/lib/python3.8/site-packages/thumbor/importer.py", line 20, in import_class
thumbor_1      |     module = import_module(module_name)
thumbor_1      |   File "/usr/local/lib/python3.8/importlib/__init__.py", line 127, in import_module
thumbor_1      |     return _bootstrap._gcd_import(name[level:], package, level)
thumbor_1      |   File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
thumbor_1      |   File "<frozen importlib._bootstrap>", line 991, in _find_and_load
thumbor_1      |   File "<frozen importlib._bootstrap>", line 961, in _find_and_load_unlocked
thumbor_1      |   File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
thumbor_1      |   File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
thumbor_1      |   File "<frozen importlib._bootstrap>", line 991, in _find_and_load
thumbor_1      |   File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
thumbor_1      |   File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
thumbor_1      |   File "<frozen importlib._bootstrap_external>", line 843, in exec_module
thumbor_1      |   File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
thumbor_1      |   File "/usr/local/lib/python3.8/site-packages/tc_aws/loaders/__init__.py", line 9, in <module>
thumbor_1      |     import urllib2
thumbor_1      | ModuleNotFoundError: No module named 'urllib2'

I guess we might need to add more libraries to requirements.txt?

tornado-botocore==1.5.0
tc-aws==6.2.10
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@heynemann I guess we still need some of the libraries here with thumbor 7 ? which ones? and which versions?

Copy link
Contributor

Choose a reason for hiding this comment

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

I created a new library called thumbor-aws we can use until tc-aws gets updated to python3. https://github.com/thumbor/thumbor-aws

The problem with urllib2 is that tc-aws is not python3 compliant. https://stackoverflow.com/questions/2792650/import-error-no-module-name-urllib2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so should we add thumbor-aws to requirements.txt ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also, correct me if I'm wrong, but wouldn't this require a change to the configs? e.g. if someone uses LOADER=tc_aws.loaders.s3_loaders they would need to change it? if so, is there a way to make thumbor-aws backwards compatible, so it accepts the "old" config directives?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, I meant as an additional lib to the docker image :) That way people who want can use it while tc-aws is not compatible.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is :) will work on making it more compatible later...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great. What's the next step here then? I'm not sure I completely follow :)

Copy link
Contributor

Choose a reason for hiding this comment

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

@heynemann is all this resolved since tc_aws 7.0.2 works with Thumbor 7?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated tc-aws to 7.0.2. There's a new thumbor 7.4.7 image on docker hub. Can you test it? ideally with tc-aws. I think this should solve the urllib2 issue.

thumbor/Dockerfile Outdated Show resolved Hide resolved
@heynemann
Copy link
Contributor

Regarding the conf file it's super fine. We didn't change any configuration during our "revamp". Added some as the handlers list, but configurations remain the same.

thumbor/requirements.txt Outdated Show resolved Hide resolved
@gingerlime
Copy link
Contributor Author

gingerlime commented Apr 28, 2023

@danquack @mpdude I think we're getting closer to getting this out. A couple of things I could really use your help on though:

  1. Testing. I don't have access to a realistic environment, so would be great if you can test it somewhere (ideally some kind of staging environment)
  2. can we improve caching/performance? the build / push is super slow, I guess multiarch takes longer? I tried using gha caching, but it doesn't feel like it has an effect. Maybe because most builds failed so far? I'm not sure. (EDIT: something feels very "off" here, building images that are identical to the ones pushed to ghcr.io start from scratch and don't seem to pull or use any cached layers)

@gingerlime
Copy link
Contributor Author

gingerlime commented Apr 29, 2023

Somehow the remotecv image isn't created/pushed. I cannot see any errors however, so it's weird. @danquack can you take a look? EDIT: should be solved now.

@gingerlime
Copy link
Contributor Author

I'll go ahead and merge these changes and move to Thumbor 7. We can deal with any issues later, and there's still the old latest and 6.x tags on docker hub anyway.

@gingerlime gingerlime merged commit d58fc82 into master May 6, 2023
1 check passed
@gingerlime gingerlime deleted the thumbor-7 branch May 6, 2023 07:34
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.

None yet

5 participants