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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed logic to get the Airflow UI link from houston #504

Merged
merged 3 commits into from
Feb 11, 2022

Conversation

neel-astro
Copy link
Contributor

Description

Changes:

  • Fixed the UI link in astro deploy output to point to airflow UI instead of astro UI

馃師 Issue(s)

Related astronomer/issues#3826

馃И Functional Testing

List the functional testing steps to confirm this feature or fix.

馃摳 Screenshots

Add screenshots to illustrate the validity of these changes.

馃搵 Checklist

  • Rebased from the main (or release if patching) branch (before testing)
  • Ran make test before taking out of draft
  • Added/updated applicable tests
  • Tested against Houston-API and Astronomer (if necessary).
  • Communicated to/tagged owners of respective clients potentially impacted by these changes.
  • Updated any related documentation

@codecov
Copy link

codecov bot commented Feb 11, 2022

Codecov Report

Merging #504 (7f08170) into release-0.28 (86d49a3) will decrease coverage by 0.11%.
The diff coverage is 55.55%.

Impacted file tree graph

@@               Coverage Diff                @@
##           release-0.28     #504      +/-   ##
================================================
- Coverage         66.28%   66.16%   -0.12%     
================================================
  Files                42       42              
  Lines              3710     3721      +11     
================================================
+ Hits               2459     2462       +3     
- Misses             1064     1070       +6     
- Partials            187      189       +2     
Impacted Files Coverage 螖
cmd/deploy.go 28.48% <45.45%> (-0.10%) 猬囷笍
cmd/airflow.go 67.91% <100.00%> (酶)
cmd/root.go 89.18% <100.00%> (酶)

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 86d49a3...7f08170. Read the comment docs.

Copy link
Contributor

@AmFlint AmFlint left a comment

Choose a reason for hiding this comment

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

lgtm :)

I didn't check if Houston returns the airflow URL for a deployment when you just created it though. Because if houston returns an empty URL and populates it later on we're gonna have a little bug.

@neel-astro
Copy link
Contributor Author

neel-astro commented Feb 11, 2022

Urls are static hence they do get generate at creation time itself and are present in create deployment call response as well

neel@Neels-MacBook-Pro airflow-test5 % ../astro deployment create neel-test-50 --executor=local --skip-version-check
 NAME             DEPLOYMENT NAME                   ASTRO     DEPLOYMENT ID                  TAG     AIRFLOW VERSION     
 neel-test-50     horizontal-radioactivity-9290               ckzil0wps2318965kptenax8b2     -       1.10.10-10          

 Successfully created deployment with Local executor. Deployment can be accessed at the following URLs 

 Airflow Dashboard: https://deployments.qa.astronomer.io/horizontal-radioactivity-9290/airflow

@neel-astro neel-astro merged commit 970f4c5 into release-0.28 Feb 11, 2022
@neel-astro neel-astro deleted the fix/0.28-airflow-ui-link branch September 26, 2022 16:15
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