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

Add action to test with upstream linkml v2 #319

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

sneakers-the-rat
Copy link
Contributor

Continues: #316

Sorry to keep doing this with CI actions - as far as I know I can't test them on the fork until they are in main?

The previous action wouldn't run because the dynamic versioning plugin can't detect version when run within a poetry shell - https://github.com/mtkennerly/poetry-dynamic-versioning?tab=readme-ov-file#caveats

you also apparently need to explicitly install the plugin.

so i modified it so that we create an arbitrarily high version with the commit hash appended to it so we can be sure that the current version is being installed rather than a cached version.

You can see that the action is running *but correctly failing due to linkml/linkml#2047 * here - https://github.com/sneakers-the-rat/linkml-runtime/actions/runs/8531972641/job/23372442173

this should get squash merged, but wanted to preserve commit history in PR so it's possible to see the various options i tried. blah. one day this will all be done and our CI will be spic and span

Copy link

codecov bot commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.06%. Comparing base (0f99716) to head (8a5dee2).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #319      +/-   ##
==========================================
+ Coverage   62.70%   64.06%   +1.36%     
==========================================
  Files          63       63              
  Lines        8580     8611      +31     
  Branches     2444     2447       +3     
==========================================
+ Hits         5380     5517     +137     
+ Misses       2583     2477     -106     
  Partials      617      617              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -24,6 +24,9 @@ jobs:
- python-version: "3.10"
pydantic-version: "1"
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's not even germane to the changes here but I recently simplified the testing matrix in the linkml repo (linkml/linkml#2041). It might be worth using the same matrix here. I wouldn't lose sleep at night if we didn't do a Pydantic v1 test in this upstream workflow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ya ya good call, i wrote this before that got merged, and we were definitely thinking the same thing ;). so would b good to match.

Comment on lines +76 to +80
- name: get linkml-runtime short hash
working-directory: linkml-runtime
run: |
LINKML_RUNTIME_COMMIT=$(git rev-parse --short HEAD)
echo "LINKML_RUNTIME_COMMIT=$LINKML_RUNTIME_COMMIT" >> "$GITHUB_ENV"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this step can be replaced with running this in the linkml-runtime directory:

poetry self add "poetry-dynamic-versioning[plugin]"
poetry dynamic-versioning

Using the dynamic versioning plugin via the CLI will modify the version field in pyproject.toml according to the current Git status and leave the change in place.

Then in the next step you should be able to just do the poetry add ../linkml-runtime. I think 😂. If that works that feels a little better to me than using the fake 1.99.0+x version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try that again, but iirc it doesn't work across directories? like dynamic versioning only works for the project you are currently modifying. but let me try again bc it also could have been that i didn't have the plugin installed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(agreed it's ugly and i hate it lol)

@sneakers-the-rat
Copy link
Contributor Author

Thinking about what we need for PRs like #314 - thats a change that also requires a change in upstream to pass ( linkml/linkml#2032 ). Is it possible/should we add some input arguments that allow this to be run against a specific version of upstream? Like I know we can add arguments, but is there a way to make that sorta streamlined - like in the OP I put some line that links to the branch of upstream to test against.

The manual version would just be putting in username, repo name and branch, with defaults for all three, which is not too bad.

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

2 participants