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

airbyte-ci: add dagger run url #28947

Merged
merged 16 commits into from
Aug 2, 2023
Merged

Conversation

pedroslopez
Copy link
Contributor

@pedroslopez pedroslopez commented Aug 1, 2023

What

Adds a link to the Dagger Cloud runs for the given commit when running in CI.

This is added to the reports as well as printed out to the console.

close #27863

How

Since we don't have a direct run ID, link to the Dagger Cloud changesByPipeline page, which lists all runs for a given commit.

Unfortunately this does mean that while the commit is the latest one on master, if anyone runs a publish or pre-release on master it will all get accumulated under that same page.

Examples

@pedroslopez pedroslopez marked this pull request as ready for review August 1, 2023 21:12
@octavia-squidington-iii

This comment was marked as outdated.

@octavia-squidington-iii

This comment was marked as outdated.

@octavia-squidington-iii

This comment was marked as outdated.

@octavia-squidington-iii

This comment was marked as outdated.

@@ -169,6 +172,6 @@
</div>
</div>
{% endfor %}
<p style="margin-top: 50px"><em>These reports are generated from <a href="https://github.com/airbytehq/airbyte/blob/master/airbyte-ci/connector_ops/connector_ops/pipelines/tests/templates/test_report.html.j2">this code</a>, please reach out to the Connector Operations team for support.</em></p>
<p style="margin-top: 50px"><em>These reports are generated from <a href="https://github.com/airbytehq/airbyte/blob/master/airbyte-ci/connectors/pipelines/pipelines/tests/templates/test_report.html.j2">this code</a>, please reach out to the Connector Operations team for support.</em></p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This link was wrong after the move

Comment on lines 632 to 635
console.print(main_panel)

if self.pipeline_context.dagger_cloud_url:
self.pipeline_context.logger.info(f"🔗 View commit in Dagger Cloud: {self.pipeline_context.dagger_cloud_url}")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally i wanted to add this as a section in the Panel printed with console.print, but for the life of me I couldn't figure out how to make the link not get cut off. In CI it kept adding a newline which made the link not clickable.

@pedroslopez pedroslopez changed the title add dagger run url airbyte-ci: add dagger run url Aug 1, 2023
@octavia-squidington-iii

This comment was marked as outdated.

@octavia-squidington-iii

This comment was marked as outdated.

@octavia-squidington-iii
Copy link
Collaborator

source-faker test report (commit e54b090911) - ❌

⏲️ Total pipeline duration: 01mn54s

Step Result
Validate airbyte-integrations/connectors/source-faker/metadata.yaml
Connector version semver check
Connector version increment check
QA checks
Code format checks
Connector package install
Build source-faker docker image for platform linux/x86_64
Unit tests
Acceptance tests

🔗 View the logs here

☁️ View commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-faker test

@pedroslopez pedroslopez requested a review from a team August 1, 2023 22:59
@octavia-squidington-iii octavia-squidington-iii removed the area/connectors Connector related issues label Aug 1, 2023
@@ -89,7 +89,7 @@ def check_dagger_cli_install() -> str:
def main():
os.environ[DAGGER_CLOUD_TOKEN_ENV_VAR_NAME_VALUE[0]] = DAGGER_CLOUD_TOKEN_ENV_VAR_NAME_VALUE[1]
exit_code = 0
if sys.argv[1] == "--no-tui":
if len(sys.argv) > 1 and sys.argv[1] == "--no-tui":
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a bug with the latest changes here when no args were passed to airbyte-ci

Copy link
Contributor

@bnchrch bnchrch left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -618,5 +628,8 @@ def print(self):
details_instructions = Text("ℹ️ You can find more details with step executions logs in the saved HTML report.")
to_render = [step_results_table, details_instructions]

if self.pipeline_context.dagger_cloud_url:
self.pipeline_context.logger.info(f"🔗 View commit in Dagger Cloud: {self.pipeline_context.dagger_cloud_url}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.pipeline_context.logger.info(f"🔗 View commit in Dagger Cloud: {self.pipeline_context.dagger_cloud_url}")
self.pipeline_context.logger.info(f"🔗 For more information and better debugging view this run in Dagger Cloud: {self.pipeline_context.dagger_cloud_url}")

Copy link
Contributor

@alafanechere alafanechere left a comment

Choose a reason for hiding this comment

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

Nice!
Only minor suggestions.

airbyte-ci/connectors/pipelines/pipelines/bases.py Outdated Show resolved Hide resolved
airbyte-ci/connectors/pipelines/pipelines/bases.py Outdated Show resolved Hide resolved
@@ -618,5 +628,8 @@ def print(self):
details_instructions = Text("ℹ️ You can find more details with step executions logs in the saved HTML report.")
to_render = [step_results_table, details_instructions]

if self.pipeline_context.dagger_cloud_url:
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't you already logging this on line 481?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a different Report's print. One is ConnectorReport.print and the other is Report.print

airbyte-ci/connectors/pipelines/pipelines/contexts.py Outdated Show resolved Hide resolved
airbyte-ci/connectors/pipelines/pipelines/contexts.py Outdated Show resolved Hide resolved
@pedroslopez pedroslopez merged commit ff2d987 into master Aug 2, 2023
17 checks passed
@pedroslopez pedroslopez deleted the pedroslopez/dagger-cloud-run-url branch August 2, 2023 20:57
bnchrch pushed a commit that referenced this pull request Aug 3, 2023
* add dagger run url

* version bump

* making a change so tests are auto triggered

* fix dagger cloud url

* fix no args

* naming / formatting

* try nowrap

* try soft_wrap

* just log it

* move log

* revert fake change

* rename

* nits

* Update airbyte-ci/connectors/pipelines/pipelines/tests/templates/test_report.html.j2

Co-authored-by: Augustin <augustin@airbyte.io>

* Update airbyte-ci/connectors/pipelines/pipelines/contexts.py

Co-authored-by: Augustin <augustin@airbyte.io>

---------

Co-authored-by: Augustin <augustin@airbyte.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

connectors-ci: The Dagger UI url is not displayed in logs
4 participants