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

reference for built-in env vars #24020

Conversation

jamiedemaria
Copy link
Contributor

@jamiedemaria jamiedemaria commented Aug 28, 2024

Summary & Motivation

Knocking out this pretty easy one too while i do the other env var doc. I have a couple aesthetic questions about how to format some things, but i think the existing doc does a pretty good job, so i ended up re-using most of it

Docs preview - https://dagster-docs-beta-5iiwhw9qp-elementl.vercel.app/dagster-plus/deployment/environment-variables/built-in

How I Tested These Changes

Changelog [New | Bug | Docs]

NOCHANGELOG

Copy link
Contributor Author

jamiedemaria commented Aug 28, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @jamiedemaria and the rest of your teammates on Graphite Graphite

Copy link

github-actions bot commented Aug 28, 2024

Deploy preview for dagster-docs ready!

Preview available at https://dagster-docs-u6gknkgdf-elementl.vercel.app
https://jamie-doc-408-refresh-dagster-built-in-environment-variables.dagster.dagster-docs.io

Direct link to changed pages:

| Key | Value |
|---|---|
| DAGSTER_CLOUD_GIT_SHA | The SHA of the commit. <br/><br/> **Example:** `5c5fa4643968a1b8043b58c159fb0600af8a35b2`. |
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'm playing around with the idea of having examples for each of the env vars. I think for some of them it's kind of redundant (like email, pr message, etc). For ones like the timestamp though, I think it is really useful as a reader to know what format you're getting the data in (YYYY-MM-DD vs MM-DD-YYYY, vs a timestamp int, etc)

Does it feel odd to have examples for some but not all? I think the timestamp var is the only one that really needs it...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea I tend to agree. I don't think they all need it. Although for Unix timestamp we should mention if its in seconds or ms, since its not always standardized.

@jamiedemaria jamiedemaria force-pushed the jamie/doc-408-refresh-dagster-built-in-environment-variables branch from e3581b3 to 751f2c2 Compare August 28, 2024 19:47
|---|---|
| DAGSTER_CLOUD_GIT_SHA | The SHA of the commit. <br/><br/> **Example:** `5c5fa4643968a1b8043b58c159fb0600af8a35b2`. |
| DAGSTER_CLOUD_GIT_TIMESTAMP | The Unix timestamp when the commit occurred. <br/><br/> **Example:** `1724871941` |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

annoyingly, on my laptop screen some of the descriptions are too long and they make the table scroll horizontally. Is there a way to make the text overflow onto a new line rather than scroll?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh that is annoying. i say leave it for now, we can fix it with better styling

@jamiedemaria jamiedemaria marked this pull request as ready for review August 28, 2024 20:32
@graphite-app graphite-app bot added area: docs Related to documentation in general docathon labels Aug 28, 2024
Copy link

graphite-app bot commented Aug 28, 2024

Graphite Automations

"docs-beta - Assign Reviewers" took an action on this PR • (08/28/24)

1 label was added and 3 reviewers were added to this PR based on Pedram Navid's automation.

@jamiedemaria jamiedemaria force-pushed the jamie/doc-410-refresh-setting-env-vars-in-the-dagster-ui branch from 5395e14 to 8f910be Compare August 29, 2024 16:20
@jamiedemaria jamiedemaria force-pushed the jamie/doc-408-refresh-dagster-built-in-environment-variables branch from 751f2c2 to c6c44e7 Compare August 29, 2024 16:20
Base automatically changed from jamie/doc-410-refresh-setting-env-vars-in-the-dagster-ui to master August 29, 2024 18:33
@jamiedemaria jamiedemaria force-pushed the jamie/doc-408-refresh-dagster-built-in-environment-variables branch from c6c44e7 to e8840e8 Compare August 29, 2024 19:19
@jamiedemaria jamiedemaria merged commit 8cdeec4 into master Aug 29, 2024
1 of 3 checks passed
@jamiedemaria jamiedemaria deleted the jamie/doc-408-refresh-dagster-built-in-environment-variables branch August 29, 2024 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: docs Related to documentation in general docathon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants