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.

Sign up for GitHub

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

Jump to bottom

Do multiple attempt to download the nodejs archive (IncompleteRead error) #329

Merged
merged 5 commits into from
May 7, 2023
Merged

Do multiple attempt to download the nodejs archive (IncompleteRead error) #329

merged 5 commits into from
May 7, 2023

Conversation

bagerard
Copy link
Contributor

@bagerard bagerard commented Mar 20, 2023 •
edited

Should fix the sporadic IncompleteRead error that we are getting when it download the nodejs archive

Relates to #324

ekalinin
ekalinin reviewed Mar 22, 2023
View reviewed changes
nodeenv.py Outdated Show resolved Hide resolved
@bagerard
Copy link
Contributor Author

bagerard commented Apr 18, 2023 •
edited

Done fixing the issues @ekalinin , please approve the workflow when you have a chance 🤞

@bagerard bagerard requested a review from ekalinin April 21, 2023 08:47
@bagerard bagerard mentioned this pull request Apr 21, 2023
Open
@bagerard
Copy link
Contributor Author

ping @ekalinin, this is really impacful for our pipelines so it'd be great if it could move forward

@nullswan
Copy link

Bump, this is happening through usage of https://github.com/RobertCraigie/prisma-client-py as well.

Alexander-Serov
Alexander-Serov approved these changes May 5, 2023
View reviewed changes
Copy link

@Alexander-Serov Alexander-Serov left a comment

Choose a reason for hiding this comment

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

Thansk for making a contribution!

nodeenv.py
'from {} - {}'.format(node_url, e)
)
n_attempt -= 1
if n_attempt == 0:
Copy link

Choose a reason for hiding this comment

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

This condition seems superflous together with the while loop. It would make sense to raise after the loop since you already return in try.

@Alexander-Serov
Copy link

@ekalinin Could you please review it? This is an important fix for us.

@ekalinin ekalinin merged commit 9f2f0c4 into ekalinin:master May 7, 2023
7 checks passed
@ekalinin
Copy link
Owner

ekalinin commented May 7, 2023

Thanks for the patch!

@fruch
Copy link

fruch commented May 10, 2023

Thanks for the patch!

It would be nice to have a release with this fix.

1 similar comment
@fruch
Copy link

fruch commented May 10, 2023

Thanks for the patch!

It would be nice to have a release with this fix.

@ekalinin
Copy link
Owner

Thanks for the patch!

It would be nice to have a release with this fix.

Done:

@fruch fruch mentioned this pull request Jul 31, 2023
Merged
7 tasks
@fruch fruch mentioned this pull request Aug 31, 2023
Open
regisb added a commit to overhangio/tutor that referenced this pull request Sep 13, 2023
@regisb
chore: upgrade nodeenv to fix nodejs install
00c58e7 
We upgrade nodeenv as an attempt to fix incomplete reads.

From time to time we face the following error:

	#67 [linux/amd64 nodejs-requirements 2/4] RUN nodeenv /openedx/nodeenv --node=16.14.0 --prebuilt
	#67 0.338  * Install prebuilt node (16.14.0) .Incomplete read while readingfrom https://nodejs.org/download/release/v16.14.0/node-v16.14.0-linux-x64.tar.gz
	#67 204.1 .
	#67 204.1 Traceback (most recent call last):
	#67 204.1   File "/openedx/venv/bin/nodeenv", line 8, in <module>
	#67 204.1     sys.exit(main())
	#67 204.1   File "/openedx/venv/lib/python3.8/site-packages/nodeenv.py", line 1104, in main
	#67 204.1     create_environment(env_dir, args)
	#67 204.1   File "/openedx/venv/lib/python3.8/site-packages/nodeenv.py", line 980, in create_environment
	#67 204.1     install_node(env_dir, src_dir, args)
	#67 204.1   File "/openedx/venv/lib/python3.8/site-packages/nodeenv.py", line 739, in install_node
	#67 204.1     install_node_wrapped(env_dir, src_dir, args)
	#67 204.1   File "/openedx/venv/lib/python3.8/site-packages/nodeenv.py", line 762, in install_node_wrapped
	#67 204.1     download_node_src(node_url, src_dir, args)
	#67 204.1   File "/openedx/venv/lib/python3.8/site-packages/nodeenv.py", line 602, in download_node_src
	#67 204.1     with ctx as archive:
	#67 204.1   File "/opt/pyenv/versions/3.8.15/lib/python3.8/contextlib.py", line 113, in __enter__
	#67 204.1     return next(self.gen)
	#67 204.1   File "/openedx/venv/lib/python3.8/site-packages/nodeenv.py", line 573, in tarfile_open
	#67 204.1     tf = tarfile.open(*args, **kwargs)
	#67 204.1   File "/opt/pyenv/versions/3.8.15/lib/python3.8/tarfile.py", line 1601, in open
	#67 204.1     saved_pos = fileobj.tell()
	#67 204.1 AttributeError: 'bytes' object has no attribute 'tell'

This change was added to 1.8.0 as an attempt to resolve the issue: ekalinin/nodeenv#329

We are not sure it will work every time, but it can't hurt.
moonesque pushed a commit to edSPIRIT/tutor that referenced this pull request Nov 20, 2023
@regisb @moonesque
chore: upgrade nodeenv to fix nodejs install
2ebd9c7 
We upgrade nodeenv as an attempt to fix incomplete reads.

From time to time we face the following error:

	overhangio#67 [linux/amd64 nodejs-requirements 2/4] RUN nodeenv /openedx/nodeenv --node=16.14.0 --prebuilt
	overhangio#67 0.338  * Install prebuilt node (16.14.0) .Incomplete read while readingfrom https://nodejs.org/download/release/v16.14.0/node-v16.14.0-linux-x64.tar.gz
	overhangio#67 204.1 .
	overhangio#67 204.1 Traceback (most recent call last):
	overhangio#67 204.1   File "/openedx/venv/bin/nodeenv", line 8, in <module>
	overhangio#67 204.1     sys.exit(main())
	overhangio#67 204.1   File "/openedx/venv/lib/python3.8/site-packages/nodeenv.py", line 1104, in main
	overhangio#67 204.1     create_environment(env_dir, args)
	overhangio#67 204.1   File "/openedx/venv/lib/python3.8/site-packages/nodeenv.py", line 980, in create_environment
	overhangio#67 204.1     install_node(env_dir, src_dir, args)
	overhangio#67 204.1   File "/openedx/venv/lib/python3.8/site-packages/nodeenv.py", line 739, in install_node
	overhangio#67 204.1     install_node_wrapped(env_dir, src_dir, args)
	overhangio#67 204.1   File "/openedx/venv/lib/python3.8/site-packages/nodeenv.py", line 762, in install_node_wrapped
	overhangio#67 204.1     download_node_src(node_url, src_dir, args)
	overhangio#67 204.1   File "/openedx/venv/lib/python3.8/site-packages/nodeenv.py", line 602, in download_node_src
	overhangio#67 204.1     with ctx as archive:
	overhangio#67 204.1   File "/opt/pyenv/versions/3.8.15/lib/python3.8/contextlib.py", line 113, in __enter__
	overhangio#67 204.1     return next(self.gen)
	overhangio#67 204.1   File "/openedx/venv/lib/python3.8/site-packages/nodeenv.py", line 573, in tarfile_open
	overhangio#67 204.1     tf = tarfile.open(*args, **kwargs)
	overhangio#67 204.1   File "/opt/pyenv/versions/3.8.15/lib/python3.8/tarfile.py", line 1601, in open
	overhangio#67 204.1     saved_pos = fileobj.tell()
	overhangio#67 204.1 AttributeError: 'bytes' object has no attribute 'tell'

This change was added to 1.8.0 as an attempt to resolve the issue: ekalinin/nodeenv#329

We are not sure it will work every time, but it can't hurt.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Alexander-Serov Alexander-Serov Alexander-Serov approved these changes

@ekalinin ekalinin Awaiting requested review from ekalinin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@bagerard @nullswan @Alexander-Serov @ekalinin @fruch