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 v1.2 Rebasing #34

Closed
wg102 opened this issue Dec 4, 2020 · 6 comments · Fixed by #36
Closed

Kubeflow v1.2 Rebasing #34

wg102 opened this issue Dec 4, 2020 · 6 comments · Fixed by #36
Assignees

Comments

@wg102
Copy link

wg102 commented Dec 4, 2020

This is to keep track of the steps for the rebasing, and the conflicts that arise.

@saffaalvi
Copy link

saffaalvi commented Dec 4, 2020

Merge (keeps our commits and history, adds upstream changes as one big merge commit with one author aka me)

  1. Clone whole repository (in this case, StatCan/kubeflow)
  2. Go into repo locally
  3. git remote add upstream https://github.com/kubeflow/kubeflow.git
  4. git remote –v (list current configured remote)
  5. git checkout stc-master
  6. git checkout –b
  7. git fetch upstream
  8. git merge upstream/v1.2-branch
  9. Resolve conflicts -> save -> stage changes
  10. git commit
  11. git push --set-upstream origin

If you want to rebase, brings in all commits from upstream, but loses our commit history (adds all commits on one date and adds as author to everything), change merge in step 8 to rebase. Branch: https://github.com/StatCan/kubeflow/tree/v1.2-branch

@wg102 wg102 self-assigned this Dec 4, 2020
@saffaalvi
Copy link

saffaalvi commented Dec 4, 2020

After discussing with the team, using git rebase is a better option since it will preserve both the kubflow upstream commit history as well as ours.

Update 12/14/2020:
Exact steps that were taken to complete this rebase:
Rebase: Brings in all of upstream's commits, preserves commit history and puts all our commits at the top of the history

1. Clone whole repository (in this case, StatCan/kubeflow) 
2. Go into repo locally 
3. git remote add upstream https://github.com/kubeflow/kubeflow.git (add upstream as remote)
4. git remote –v (list current configured remote) 
5. git checkout stc-master 
6. git checkout –b <new-branch-name> 
7. git fetch upstream 
8. git rebase upstream/v1.2-branch 
9. Resolve conflicts -> save -> stage changes 
10. git commit -m <commit message>
11. git push --set-upstream origin <new-branch-name> 

We chose to create a feature branch and rebased that branch to v1.2-branch in upstream kubeflow/kubeflow repository. This feature branch was then merged back into stc-master.

Our feature branch was named v1.2-branch, the same name as the branch we were rebasing off upstream. We suggest creating a branch with a different name from upstream, since this might get confusing later as the branch is actually linked to upstream.

Another thing to note is while this method preserved all the dates of our commit, it also added them on top of the commit history. So, when comparing stc-master with upstream kubeflow v1.2-branch, the number of commits we were ahead of them by was doubled.

@saffaalvi
Copy link

Branch https://github.com/StatCan/kubeflow/tree/v1.2-branch has been rebased to the v1.2-branch upstream.
Conflicts that came up while rebasing, which we tried to resolve but are going to take another look:

Central Dashboard now as configmap
Upstream commit: kubeflow@ec7e01a#diff-a29b81659d6ca88b1df24f77d04886f8eb04631f08a34e41da53e6bb273e3be0
Will have to go back and integrate these changes with our links, documentation, and restructure.

i18n (en/fr)
Our commit: f85a597
Upstream has no translations at all, so we have to fit this in with their new configmap.

Security Update
Our commit: 3011617#diff-a29b81659d6ca88b1df24f77d04886f8eb04631f08a34e41da53e6bb273e3be0
We changed for security vulnerabilities:
coreAPI: k8s.Core_v1Api; to k8s.CoreV1Api;
customObjectsAPI: k8s.Custom_objectsApi; to k8s.CustomObjectsApi;
Upstream has not updated this yet.

@wg102
Copy link
Author

wg102 commented Dec 4, 2020

We need to find a way to test central dashboard. Since the links are now coming from configmap, and it's an API call '/dashboard-links' the local npm run dev now shows an almost empty page.
There are 4 errors like these:

Failed to load resource: the server responded with a status of 504 (Gateway Timeout)

@saffaalvi
Copy link

We ran into an issue where there was an error retrieving the pipelines. Zach fixed the error in this commit: 5a07fb4 and Wendy fixed the pipeline tests here: 77c965c. Not sure if this is a problem upstream but they didn't seem to have any visible fix for it.

@wg102
Copy link
Author

wg102 commented Dec 11, 2020

We ran into the issue of the configmap. The menu links, quick links and documentation links now come from a centraldashboard-links config map which we were not able to customize to our docs directly from changing the file. It seems like a different resource when looking at the Network tab. The resource it uses is viewable with this command : kubectl -n kubeflow get configmap centraldashboard-links-config This will need to be addressed. For now, we removed the code that was using it and kept the arrays that were previously used.

@wg102 wg102 mentioned this issue Dec 11, 2020
saffaalvi pushed a commit that referenced this issue Feb 5, 2021
fix: add check for empty response
saffaalvi pushed a commit that referenced this issue Feb 12, 2021
fix: add check for empty response
saffaalvi pushed a commit that referenced this issue Feb 12, 2021
fix: add check for empty response
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants