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

added classes for proof and cromwell apis; first wdl submit test #30

Merged
merged 8 commits into from
Dec 18, 2024

Conversation

sckott
Copy link
Collaborator

@sckott sckott commented Dec 17, 2024

No description provided.

@sckott sckott requested review from seankross and tefirman December 17, 2024 20:51
@seankross
Copy link
Collaborator

@sckott weird question but is it possible/ would it be appropriate to test some of this stuff locally? Presumably I would just need the right env vars and to be on the fh network, right?

@sckott
Copy link
Collaborator Author

sckott commented Dec 17, 2024

@seankross not a weird question. sure, I run it locally. I think we should support locally running tests too. Yeah, so:

  • be on a network where you can hit proof and cromwell apis
  • have the PROOF_API_TOKEN_DEV token

- rename constants file to utils
- define only base url for proof api, not cromwell in utils
- confest now has setup fixture to submit all wdls in the repo
- add retry metadata method to cromwell class
- rename api-basics test file to version
- now using tenacity for retry behavior
@seankross
Copy link
Collaborator

After 30 min of trying to get this to work with the wrong token, I read the instructions and used the right token, and it works well! At least I got to pick it apart for real. Thanks so much for this Scott.

@sckott
Copy link
Collaborator Author

sckott commented Dec 18, 2024

Great, so any thoughts on it or good to merge?

@seankross
Copy link
Collaborator

I think it looks great, it's parsimonious, I only have praise to offer unless you're looking for a specific feedback. If you feel good, go ahead and merge.

@sckott
Copy link
Collaborator Author

sckott commented Dec 18, 2024

lets get it merged, then i can work on documenting it and other PRs can be worked on

@sckott sckott merged commit 8c18e24 into main Dec 18, 2024
1 check passed
@sckott sckott deleted the api-tests-first-wdl branch December 18, 2024 18:49
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.

2 participants