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

chore: add latest tag action #11148

Merged
merged 7 commits into from
Dec 17, 2020

Conversation

eschutho
Copy link
Member

@eschutho eschutho commented Oct 3, 2020

SUMMARY

Action that adds a "latest" tag to release. It also overwrites/removes the last "latest" tag.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@eschutho eschutho changed the title add latest tag action [WIP] chore: add latest tag action Oct 3, 2020
@eschutho eschutho marked this pull request as draft October 3, 2020 00:31
@eschutho eschutho changed the title [WIP] chore: add latest tag action chore: add latest tag action Oct 3, 2020
@eschutho eschutho marked this pull request as ready for review October 3, 2020 00:37
Copy link
Member

@dpgaspar dpgaspar left a comment

Choose a reason for hiding this comment

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

Cool automation! maybe drop a comment that this will automatically run on https://github.com/apache/incubator-superset/blob/master/RELEASING/README.md and a different one superset docs/instalation

@eschutho
Copy link
Member Author

eschutho commented Oct 6, 2020

Cool automation! maybe drop a comment that this will automatically run on https://github.com/apache/incubator-superset/blob/master/RELEASING/README.md and a different one superset docs/instalation

good idea. I had an earlier PR with docs/instructions to check out the latest before this tag that was blocked, so I just included it in here.

@codecov-io
Copy link

codecov-io commented Oct 7, 2020

Codecov Report

Merging #11148 (b6a7ef1) into master (e0288bf) will decrease coverage by 12.79%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #11148       +/-   ##
===========================================
- Coverage   67.68%   54.89%   -12.80%     
===========================================
  Files         930      421      -509     
  Lines       45132    14810    -30322     
  Branches     4331     3821      -510     
===========================================
- Hits        30549     8130    -22419     
+ Misses      14480     6680     -7800     
+ Partials      103        0      -103     
Flag Coverage Δ
cypress 54.89% <ø> (ø)
javascript ?
python ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...uperset-frontend/src/dashboard/util/dnd-reorder.js 0.00% <0.00%> (-100.00%) ⬇️
...rset-frontend/src/dashboard/util/getEmptyLayout.js 0.00% <0.00%> (-100.00%) ⬇️
...dashboard/components/resizable/ResizableHandle.jsx 0.00% <0.00%> (-100.00%) ⬇️
.../src/dashboard/util/getFilterScopeFromNodesTree.js 0.00% <0.00%> (-93.48%) ⬇️
...src/dashboard/components/gridComponents/Header.jsx 10.52% <0.00%> (-86.85%) ⬇️
...rc/dashboard/components/gridComponents/Divider.jsx 13.33% <0.00%> (-86.67%) ⬇️
...ontend/src/dashboard/util/getDashboardFilterKey.ts 14.28% <0.00%> (-85.72%) ⬇️
...nd/src/views/CRUD/data/query/QueryPreviewModal.tsx 14.70% <0.00%> (-82.97%) ⬇️
...set-frontend/src/views/CRUD/welcome/EmptyState.tsx 5.71% <0.00%> (-82.10%) ⬇️
...et-frontend/src/SqlLab/components/TableElement.jsx 4.70% <0.00%> (-81.35%) ⬇️
... and 777 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0288bf...b6a7ef1. Read the comment docs.

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM. My only worry is the scenario where we might do a fix to an old version (say 0.37.3) after we've released a more recent major/minor 0.38.0. In that case I believe this would point latest to 0.37.3. I think this won't be a problem right now, but once we hit 1.0, we'll probably start having this problem when we start releasing new majors while maintaining minors for stability.

docs/src/pages/docs/installation/installing_scratch.mdx Outdated Show resolved Hide resolved
@eschutho
Copy link
Member Author

eschutho commented Oct 8, 2020

LGTM. My only worry is the scenario where we might do a fix to an old version (say 0.37.3) after we've released a more recent major/minor 0.38.0. In that case I believe this would point latest to 0.37.3. I think this won't be a problem right now, but once we hit 1.0, we'll probably start having this problem when we start releasing new majors while maintaining minors for stability.

That's a good point. Let me see if there's a check I can do in this action to make this more accurate.

@eschutho eschutho marked this pull request as draft October 8, 2020 01:30
@pull-request-size pull-request-size bot added size/L and removed size/M labels Oct 9, 2020
@eschutho eschutho marked this pull request as ready for review October 9, 2020 23:57
@eschutho eschutho force-pushed the elizabeth/latest-tag-action branch 2 times, most recently from 7101943 to ceb8e43 Compare October 10, 2020 00:16
@eschutho
Copy link
Member Author

@villebro I updated the logic in the action to see if the new release is tagged with a later version, if you want to take another look. thanks!

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

A few comments. I wonder if this could be changed into a bash script that could also be run from the command for testing? There's lots of really helpful echos here that could be helpful when run locally when further developing this functionality.

- name: Check for latest tag
id: latest-tag
run: |
LATEST_TAG=$(git show-ref --tags -d | grep latest^{} | sed 's/refs\/tags\/latest^{}//') || echo 'not found'
Copy link
Member

Choose a reason for hiding this comment

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

It took me some time to figure out what ^{} means in this context (I thought it was some non-standard regex in grep 😆 ). Perhaps we could add a comment here explaining what we're greping and seding.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wound up changing the lookup on this, so that we didn't have to look for the tag with the pointer sha. It should read a bit better now.

LATEST_RC_TAG_SPLIT=(${LATEST_RC_TAG})

## remove the 'rc' from the current tag if it exists and split into an array at the dot
THIS_TAG=($(echo ${{ github.event.release.tag_name }} | sed 's/rc//')) || echo 'not found'
Copy link
Member

Choose a reason for hiding this comment

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

We probably don't want to consider release candidates for latest?

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks.. I removed them.

@eschutho eschutho marked this pull request as ready for review December 4, 2020 00:35
@eschutho
Copy link
Member Author

eschutho commented Dec 4, 2020

@villebro this should be ready for another look. I pulled the script out of the github action, so it can be run separate, with or without a --dry-run flag. You can also use a tool like nektos/act to run the github action directly. I could potentially add info into the readme on that, or we can leave as is. Either way, lmk what you think, especially if you get a chance to test this out!

@eschutho eschutho marked this pull request as draft December 4, 2020 17:59
@eschutho
Copy link
Member Author

eschutho commented Dec 4, 2020

Found one more bug I want to test...

@eschutho eschutho marked this pull request as ready for review December 11, 2020 00:23
@eschutho
Copy link
Member Author

I tested a few more times, and it's looking stable so far. Maybe we should consider some bash automated testing systems?

description: Superset latest release
tag-name: latest
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Copy link
Member Author

Choose a reason for hiding this comment

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

@ktmud may have some insight on using secrets.GITHUB_TOKEN vs github.GITHUB_TOKEN. I haven't seen the latter in documentation, although it seems that using it over the secrets could be worth looking at.

Copy link
Member

@ktmud ktmud Dec 11, 2020

Choose a reason for hiding this comment

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

It seems there is no github.GITHUB_TOKEN available according to the doc, but github.token is equivalent to secrets.GITHUB_TOKEN: https://docs.github.com/en/free-pro-team@latest/actions/reference/context-and-expression-syntax-for-github-actions#github-context

Copy link
Member Author

Choose a reason for hiding this comment

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

great. thanks for that. That makes sense.

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM!

@eschutho
Copy link
Member Author

@villebro I caught another small bug during demo. I'll put up a fix today.

@eschutho eschutho marked this pull request as draft December 14, 2020 18:26
@eschutho eschutho marked this pull request as ready for review December 14, 2020 18:46
@eschutho
Copy link
Member Author

Ok, just one more small change ba31898 It shouldn't affect the rest of the functionality, it's just one more check to be sure that the tag passed in exists. It won't be an issue when using the action itself, but could happen when someone uses the script directly.

@eschutho
Copy link
Member Author

@villebro @dpgaspar I think this is ready on my end. If everything looks good to you, you can merge when ready. Thanks for all your help!

@eschutho
Copy link
Member Author

When we land this, we should create a latest tag on 0.38.0.

@villebro villebro merged commit 895fa19 into apache:master Dec 17, 2020
@villebro villebro deleted the elizabeth/latest-tag-action branch December 17, 2020 18:52
amitmiran137 pushed a commit to simcha90/incubator-superset that referenced this pull request Dec 18, 2020
* upstream/master: (55 commits)
  feat(explore): time picker enhancement (apache#11418)
  feat: update alert/report icons and column order (apache#12081)
  feat(explore): metrics and filters controls redesign (apache#12095)
  feat(alerts/reports): add refresh action (apache#12071)
  chore: add latest tag action (apache#11148)
  fix(reports): increase crontab size and alert fixes (apache#12056)
  Small typo fix in Athena connection docs (apache#12099)
  feat(queries): security perm simplification (apache#12072)
  feat(databases): security perm simplification (apache#12036)
  feat(dashboards): security permissions simplification (apache#12012)
  feat(logs): security permissions simplification (apache#12061)
  chore: Remove unused CodeModal (apache#11972)
  Fix typescript error (apache#12074)
  fix: handle context-dependent feature flags in CLI (apache#12088)
  fix: Fix "View in SQLLab" bug (apache#12086)
  feat(alert/report): add 'not null' condition option to modal (apache#12077)
  bumping superset ui to 15.18 and deckgl to 0.3.2 (apache#12078)
  fix: Python dependencies in apache#11499 (apache#12079)
  reset active tab on open (apache#12048)
  fix: improve import flow UI/UX (apache#12070)
  ...
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L 🚢 1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants