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

test: Test cos #46

Closed
wants to merge 11 commits into from
Closed

Conversation

cademirch
Copy link
Contributor

Adds "-e PYTHONUNBUFFERED=1" to snakemake container options to flush snakemake stdout/stderr so they show up in google logs

johanneskoester and others added 11 commits March 12, 2024 10:55
🤖 I have created a release *beep* *boop*
---


##
[0.3.1](snakemake/snakemake-executor-plugin-googlebatch@v0.3.0...v0.3.1)
(2024-03-12)


### Bug Fixes

* update to latest interface
([snakemake#30](snakemake#30))
([af536dc](snakemake@af536dc))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Problem: newer version of python (via conda) missing datrie (the wheel
fails to build)
Solution: install datrie with conda

This should fix snakemake#36

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
Co-authored-by: vsoch <vsoch@users.noreply.github.com>
🤖 I have created a release *beep* *boop*
---


##
[0.3.2](snakemake/snakemake-executor-plugin-googlebatch@v0.3.1...v0.3.2)
(2024-04-05)


### Bug Fixes

* ensure we install datrie
([snakemake#37](snakemake#37))
([62f7404](snakemake@62f7404))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…nakemake#41)

Should fix snakemake#39. Previously snakemake#40

Co-authored-by: Cade Mirchandani <cmirchan@ucsc.edu>
🤖 I have created a release *beep* *boop*
---


##
[0.3.3](snakemake/snakemake-executor-plugin-googlebatch@v0.3.2...v0.3.3)
(2024-04-06)


### Bug Fixes

* make get_snakefile return rel path to snakefile
([snakemake#40](snakemake#40))
([snakemake#41](snakemake#41))
([de2b1da](snakemake@de2b1da))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
I have been able to add batch COS as suggested to run a hello world
workflow, but now the original workflows are no longer running,
and there is not sufficient error message in the log beyond
WorkflowError to understand what is happening.

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@cademirch cademirch mentioned this pull request Apr 8, 2024
@cademirch cademirch changed the title Test cos test: Test cos Apr 8, 2024
@cademirch cademirch mentioned this pull request Apr 8, 2024
@cademirch
Copy link
Contributor Author

Oops I am trying to pull this into main. I can change that...

@vsoch vsoch changed the base branch from main to testing-cos April 8, 2024 23:41
@vsoch
Copy link
Collaborator

vsoch commented Apr 8, 2024

@cademirch this is the correct branch to PR to - either you can keep your branch (but use commits from the other so they are properly attributed) or rebase in this context. What I discourage is to take someone else's code and commit/present it as your own.

@vsoch
Copy link
Collaborator

vsoch commented Apr 8, 2024

And if you intended to rebase against main, I don't think you did that because changes from the testing-cos branch were present!

@cademirch
Copy link
Contributor Author

cademirch commented Apr 8, 2024

@vsoch Agh, sorry about that. Definitely did not intend and don't want to take credit for other's commits! For my sake, how should have I done this? My quick google led me to do this:

git checkout -b test--cos upstream/testing-cos
git rebase main
... add my changes, push to my fork, open PR

@vsoch
Copy link
Collaborator

vsoch commented Apr 9, 2024

My guess is that your main branch has commits not present here, meaning at some point you did a PR against snakemake from your main branch that was squashed (and now you have extra commits).

What you want to do to fix this is to clone snakemake fresh, force push the main branch to your main, and then moving forward never do a PR from your main to snakemake main. Always work from feature branches.

@vsoch
Copy link
Collaborator

vsoch commented Apr 9, 2024

Also if you are new to rebasing, I made a very dumb video a few years ago, haha. https://youtu.be/9F4RE2_yn6I

But rebasing only works if your main is in sync with the upstream's main!

@cademirch
Copy link
Contributor Author

Awesome. Thank you for explaining this and bearing with my lack of git skills. Weirdly my main branch is not ahead of this one - so I'm not even sure what I did, but some how messed up anyways 😅. If it's okay, I'll close this and make a new draft. Quick question - do I even need to rebase against main? Or will you able to rebase when you merge testing-cos into main?

@vsoch
Copy link
Collaborator

vsoch commented Apr 9, 2024

Quick question - do I even need to rebase against main? Or will you able to rebase when you merge testing-cos into main?

If you checkout your new feature branch directly from main you should not need to! Rebasing is only when you have commits you want to fixup or squash relative to some other branch, or you pulled an update upstream main to your main, and want to rebase to ensure the changes there are in your feature branch.

@cademirch
Copy link
Contributor Author

I guess I'm confused since if I checkout from main I wouldn't have the testing-cos code since its not in main?

Sorry for all the questions. I've created a draft #47, which just adds my two changes on top of the testing-cos branch, which I think is appropriate.

@vsoch
Copy link
Collaborator

vsoch commented Apr 9, 2024

I guess I'm confused since if I checkout from main I wouldn't have the testing-cos code since its not in main?

Every new feature branch is generally checked out from main, but you are right if you are doing a PR to testing-cos it's ok to checkout from there too.

@cademirch cademirch closed this Apr 9, 2024
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

3 participants