Conversation
boring-cyborg
bot
added
area:providers
provider:common-compat
provider:edge
…4/airflow into Fix_version_check_#61313
|
(1) the package-lock.json should not be committed. You can setup your devenv as described in https://github.com/apache/airflow/blob/main/contributing-docs/03a_contributors_quick_start_beginners.rst and with prek the static assets are locally re-generated including the needed www-hash.txt (2) Please check the PR template banner and use the template (3) I assume the fix is not working by reading the code. Please check with the commands given in the issue title included if it really fixes. I do not see how a pre-release suffix shall be cut and frankly speaking the fix looks like AI slop |
Ajay9704
force-pushed
the
Fix_version_check_#61313
branch
from
5dd5dfc to
ed92ce1
Compare
Ajay9704
force-pushed
the
Fix_version_check_#61313
branch
from
c4cc99a to
7bee3e8
Compare
jscheffl
left a comment
jscheffl
left a comment
There was a problem hiding this comment.
Note that static checks fail most probably because you do not have "prek" installed as static check tool locally. This would run needed linter and compile assets in a way as needed.
Can you please follow https://github.com/apache/airflow/blob/main/contributing-docs/03a_contributors_quick_start_beginners.rst ?
Otherwise if I take a look to the diff a couple of unrelated changes on the Nav layout existing, not sure if this is an artefact of testing, should be removed/reverted.
| expect(parseVersion("3.2.0rc1")).toBe("3.2.0"); | ||
| }); | ||
|
|
||
| it("handles pre-release versions with dash separator", () => { |
There was a problem hiding this comment.
We do not use versions other than "rcN", also not with dashes. So these tests can be removed.
Unrelated to current PR but related to the release process. I submitted another PR and noticed that the www-hash.txt that was generated on my laptop differed from the one generated by CI / CD tool. I assumed it might be related to using mac and linux on ci/cd. Does that make sense? Here is the PR I am taking about: #60908 |
Should not differ. But not tamper proof. Most developers work with Linux and/or MasOS. So might be you were hitting a but. But hopefully static building of assets will be removed soon, still working on it in #56456 |
3 participants
What does this PR do?
Fixes the version check in the EdgeExecutor UI where Airflow versions like
3.1.7rc1or other pre-release identifiers were not handled correctly.These versions should be treated the same as their base version (e.g.
3.1.7rc1→3.1.7) for the UI navigation logic.Why is this needed?
The UI incorrectly determined the router type for release-candidate versions
of Airflow, causing broken navigation under the EdgeExecutor tab.
How does this fix it?
The version string returned by
/api/v2/versionis normalized usingsemver.valid()orsemver.clean()to coerce(data.version)Note
Used AI only for some parts of code formatting and conflict resolution,
as required by ASF guidelines.