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

Kubeflow 1.3 Upgrade: Local testing #838

Closed
skyeturriff opened this issue Jan 26, 2022 · 5 comments · Fixed by StatCan/aaw-kubeflow-manifests#151 or StatCan/kubeflow#77
Closed

Kubeflow 1.3 Upgrade: Local testing #838

skyeturriff opened this issue Jan 26, 2022 · 5 comments · Fixed by StatCan/aaw-kubeflow-manifests#151 or StatCan/kubeflow#77
Assignees
Labels
size/M 2-3 days

Comments

@skyeturriff
Copy link

Local testing before getting it into dev.

Linked with #833

We're doing it as a separate issue as more effort is required to investigate why build tests are failing, and to build a successful image for deployment into local kubeflow cluster (following these instructions)

@skyeturriff skyeturriff self-assigned this Jan 26, 2022
@skyeturriff skyeturriff added the size/M 2-3 days label Jan 26, 2022
@skyeturriff
Copy link
Author

Running npm test fails with the error
image
on merge branch, but not on upstream's v1.3 branch.

Running npx eslint --print-config main-page.js on both produces vastly different eslint configs. Namely of note on merge branch, the resulting config file includes:

"no-prototype-builtins": [
      "error"
],

While on upstream, the resulting config file has:

"no-prototype-builtins": "off",

The project's eslintrc.json config files (found in the centraldashboard/ and centraldashboard/public/ directories) on the merge branch and the upstream branch are identical.

@skyeturriff
Copy link
Author

Changing eslint version in package.json to match the versions used on upstream's v1.3 branch allows npm test to compile successfully, and results in npx eslint --print-config main-page.js generating an identical config to that of upstream

Versions that appeared on merge branch after the merge, where npm test fails:

"eslint": "^7.7.0",
"eslint-config-google": "^0.12.0",
"eslint-loader": "^4.0.2",
"eslint-plugin-jasmine": "^2.10.1",

Versions on upstream v1.3 branch, when used on merge branch allows npm test to succeed:

"eslint": "^5.16.0",
"eslint-config-google": "^0.12.0",
"eslint-loader": "^2.2.1",
"eslint-plugin-jasmine": "^2.10.1",

@wg102
Copy link
Contributor

wg102 commented Jan 31, 2022

Some tests needed to be adjusted following kubeflow/kubeflow@876016a

Tehre is still one test failing, the activities-litst_test.js Show Activities in descending by lastTimesStamp

@skyeturriff
Copy link
Author

Tests pass and image builds!
image

@skyeturriff
Copy link
Author

skyeturriff commented Feb 2, 2022

Build image has issues with i18n. Causing errors with the menuLinks.

Meanwhile, TODOs (from parent):

  • Investigate if Central Dashboard Controller is a different offshoot of upstream's and port over any changes (also look into Manage Contributors)
  • Confirm changes from March 24 commit are implemented in our fork ( feat(profile-controller): add pipelines.kubeflow.org/enabled label to namespaces # 5762 )

To look for in the UI walkthrough (once testable image is available):

  • i18n (test in french as well)
  • Manage Contributors => also good idea to create second namespace and test this
  • Menu items (Make sure current page is bolded in side menu (main-page.pug))
  • Documentation items
  • Remove Volume Web Apps from side menu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment