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

Update Python version at the build_windows dockerfile #7072

Conversation

mrpau-richard
Copy link
Contributor

Summary

I updated the Python version at the windows build dockerfile

Reviewer guidance

Test the Windows installer built in this PR from the BuildKite.

References


Contributor Checklist

PR process:

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Testing:

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

Reviewer Checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@codecov
Copy link

codecov bot commented Jun 17, 2020

Codecov Report

Merging #7072 into release-v0.14.x will decrease coverage by 0.30%.
The diff coverage is n/a.

Impacted Files Coverage Δ
kolibri/plugins/user/assets/src/views/AuthBase.vue 51.35% <0.00%> (-7.27%) ⬇️
kolibri/core/device/hooks.py 83.33% <0.00%> (-6.67%) ⬇️
kolibri/core/content/serializers.py 90.05% <0.00%> (-3.89%) ⬇️
kolibri/core/device/middleware.py 92.68% <0.00%> (-3.87%) ⬇️
kolibri/core/tasks/utils.py 89.65% <0.00%> (-3.45%) ⬇️
kolibri/core/assets/src/core-app/client.js 12.24% <0.00%> (-2.76%) ⬇️
kolibri/core/device/permissions.py 80.76% <0.00%> (-1.84%) ⬇️
...libri/core/auth/management/commands/deprovision.py 91.89% <0.00%> (-1.29%) ⬇️
kolibri/core/content/models.py 87.50% <0.00%> (-1.23%) ⬇️
kolibri/core/auth/api.py 87.81% <0.00%> (-0.48%) ⬇️
... and 35 more

@cpauya
Copy link
Contributor

cpauya commented Jun 17, 2020

Looking good so far! Will review a while then approve this to get it merged.
Screen Shot 2020-06-17 at 3 32 38 PM

@cpauya
Copy link
Contributor

cpauya commented Jun 17, 2020

@mrpau-richard Can you take a look why it's still extracting python-setup\python-3.4.3.msi?
Screen Shot 2020-06-17 at 3 38 28 PM

@cpauya
Copy link
Contributor

cpauya commented Jun 17, 2020

Python 3.4.3 packages are still included in the installer.

Let's remove them @mrpau-richard and this is good to go! 😸
Screen Shot 2020-06-17 at 3 44 47 PM

@cpauya
Copy link
Contributor

cpauya commented Jun 17, 2020

Can you also update this Makefile? https://github.com/learningequality/kolibri-installer-windows/blob/v1.4.0-beta1/src/Makefile#L2 - I assume it's the one downloading those python-3.4.3.* files.

@cpauya cpauya modified the milestone: 0.14.0 Jun 17, 2020
@mrpau-richard
Copy link
Contributor Author

Can you also update this Makefile? https://github.com/learningequality/kolibri-installer-windows/blob/v1.4.0-beta1/src/Makefile#L2 - I assume it's the one downloading those python-3.4.3.* files.

Thanks @cpauya , I'll create another beta release for that changes in the Windows installer repo.

@cpauya
Copy link
Contributor

cpauya commented Jun 17, 2020

For Reference, this fixes this issue: learningequality/kolibri-installer-windows#171

@cpauya
Copy link
Contributor

cpauya commented Jun 17, 2020

Can you also update this Makefile? https://github.com/learningequality/kolibri-installer-windows/blob/v1.4.0-beta1/src/Makefile#L2 - I assume it's the one downloading those python-3.4.3.* files.

Thanks @cpauya , I'll create another beta release for that changes in the Windows installer repo.

Nope, it should be changed in this PR to prevent downloading python packages twice.

That Makefile is being called via the make command here https://github.com/learningequality/kolibri/pull/7072/files#diff-6a804582ce0ce188519a48b7721c5da9R37 - It's the script downloading those python-3.4.3.* files as per BuildKite logs: https://buildkite.com/learningequality/kolibri-python-package/builds/1131#93e2bb69-0530-4703-bc9b-111af5e5173a/163-220

And then the curl command gets called next which downloads the python-3.6.8.* files.

I think doing either of this will work fine:

  1. You can make the changes to python-3.6.8 in the Makefile and remove the curl commands;
  2. Comment-out (let's not remove them yet) the wget commands in the Makefile.

Please check.

@cpauya
Copy link
Contributor

cpauya commented Jun 17, 2020

Ah, I see you already made changes at https://github.com/learningequality/kolibri-installer-windows/pull/172/files -- please ignore my message above then. 😸

Remove curl downloading Python installer in the dockerfile
@mrpau-richard
Copy link
Contributor Author

Ah, I see you already made changes at https://github.com/learningequality/kolibri-installer-windows/pull/172/files -- please ignore my message above then.

Yeah, I already made the v1.4.0-beta2 , Please test the next Windows installer built in this PR.

@mrpau-julius
Copy link
Contributor

@mrpau-richard, @cpauya is right. We missed to change the Makefile. The python version are still in 3.4.3. It's just need to change to version 3.6.8 to make it work. Make.bat is use to local build only.

@cpauya
Copy link
Contributor

cpauya commented Jun 17, 2020

@mrpau-richard You did not update the checksum values of the python-3.6.8-*.exe files - BuildKite is failing https://buildkite.com/learningequality/kolibri-python-package/builds/1132#f1c50507-48cf-4c63-85c6-1f6ec2b41674/164-1481
Screen Shot 2020-06-17 at 4 51 26 PM

The checksums are here: https://www.python.org/downloads/release/python-368/

@cpauya
Copy link
Contributor

cpauya commented Jun 17, 2020

The artifact on latest build at BuildKite for this PR now works! We can now merge this to get the fixes into the release-v0.14.x branch.
Screen Shot 2020-06-17 at 6 42 22 PM

Thanks @mrpau-richard!

/cc: @radinamatic

Copy link
Contributor

@cpauya cpauya left a comment

Choose a reason for hiding this comment

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

All good! Artifact from BuildKite tested on local Windows7 VM.

@cpauya
Copy link
Contributor

cpauya commented Jun 17, 2020

And just to double-check, there's no more python-3.4.3-* files included in the package. 😸
Screen Shot 2020-06-17 at 6 48 05 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants