Skip to content

Commit

Permalink
[OSS Nux] Add kill switch + persist Nux dismissal in temp folder (#11758
Browse files Browse the repository at this point in the history
)

### Summary & Motivation

Couple of changes:

1. Only show the Nux if you're on localhost
2. Persist that we dismissed the nux in the directory we use for
telemetry too
3. Add killswitch to completely disable this

For the kill switch in dagser.yaml we need to add
```
nux:
  enabled: False
```


### How I Tested These Changes

Unit tests for the graphql

Made sure the file was persisted and stayed past restarts but wasn't
sure how to effectively test this in python.

<img width="770" alt="Screen Shot 2023-01-17 at 5 58 23 PM"
src="https://user-images.githubusercontent.com/2286579/213030927-26d05434-eb6c-4015-8a84-6f0255340e53.png">
  • Loading branch information
salazarm committed Jan 18, 2023
1 parent 85d0357 commit 1195798
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 3 deletions.
32 changes: 29 additions & 3 deletions js_modules/dagit/packages/app/src/NUX/CommunityNux.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {gql, useMutation, useQuery} from '@apollo/client';
import {useStateWithStorage} from '@dagster-io/dagit-core/hooks/useStateWithStorage';
import {
Body,
Expand All @@ -16,17 +17,25 @@ import React from 'react';
import isEmail from 'validator/lib/isEmail';

export const CommunityNux = () => {
const [didDismissCommunityNux, dismissCommunityNux] = useStateWithStorage(
const [didDismissCommunityNux, dissmissInBrowser] = useStateWithStorage(
'communityNux',
(data) => data,
);
if (didDismissCommunityNux) {
const {data, loading} = useQuery(GET_SHOULD_SHOW_NUX_QUERY);
const [dismissOnServer] = useMutation(SET_NUX_SEEN_MUTATION);

if (!isLocalhost()) {
// Yes, we only want to show this on localhost for now.
return null;
}
if (didDismissCommunityNux || loading || (data && !data.shouldShowNux)) {
return null;
}
return (
<CommunityNuxImpl
dismiss={() => {
dismissCommunityNux('1');
dissmissInBrowser('1');
dismissOnServer();
}}
/>
);
Expand Down Expand Up @@ -212,3 +221,20 @@ const RecaptchaIFrame: React.FC<{
};

const IFRAME_SRC = '//dagster.io/dagit_iframes/community_nux';

const SET_NUX_SEEN_MUTATION = gql`
mutation SetNuxSeen {
setNuxSeen
}
`;

const GET_SHOULD_SHOW_NUX_QUERY = gql`
query ShouldShowNux {
shouldShowNux
}
`;

function isLocalhost() {
const origin = window.location.origin;
return origin.startsWith('http://127.0.0.1') || origin.startsWith('http://localhost');
}
2 changes: 2 additions & 0 deletions js_modules/dagit/packages/core/src/graphql/schema.graphql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions js_modules/dagit/packages/core/src/graphql/types.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import dagster._check as check
import graphene
from dagster._core.definitions.events import AssetKey
from dagster._core.nux import get_has_seen_nux, set_nux_seen
from dagster._core.workspace.permissions import Permissions

from dagster_graphql.implementation.execution.backfill import (
Expand Down Expand Up @@ -619,6 +620,20 @@ def mutate(self, graphene_info, **kwargs):
return action


class GrapheneSetNuxSeenMutation(graphene.Mutation):
"""Store whether we've shown the nux to any user and they've dismissed or submitted it"""

Output = graphene.NonNull(graphene.Boolean)

class Meta:
name = "SetNuxSeenMutation"

@capture_error
def mutate(self, _graphene_info):
set_nux_seen()
return get_has_seen_nux()


class GrapheneDagitMutation(graphene.ObjectType):
"""The root for all mutations to modify data in your Dagster instance."""

Expand Down Expand Up @@ -646,3 +661,4 @@ class Meta:
resume_partition_backfill = GrapheneResumeBackfillMutation.Field()
cancel_partition_backfill = GrapheneCancelBackfillMutation.Field()
log_telemetry = GrapheneLogTelemetryMutation.Field()
set_nux_seen = GrapheneSetNuxSeenMutation.Field()
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
ScheduleSelector,
SensorSelector,
)
from dagster._core.nux import get_has_seen_nux
from dagster._core.scheduler.instigation import InstigatorType

from dagster_graphql.implementation.fetch_logs import get_captured_log_metadata
Expand Down Expand Up @@ -409,6 +410,11 @@ class Meta:
description="Captured logs are the stdout/stderr logs for a given log key",
)

shouldShowNux = graphene.Field(
graphene.NonNull(graphene.Boolean),
description="Whether or not the NUX should be shown to the user",
)

def resolve_repositoriesOrError(self, graphene_info, **kwargs):
if kwargs.get("repositorySelector"):
return GrapheneRepositoryConnection(
Expand Down Expand Up @@ -723,3 +729,6 @@ def resolve_capturedLogs(self, graphene_info, logKey, cursor=None, limit=None):
logKey, cursor=cursor, max_bytes=limit
)
return from_captured_log_data(log_data)

def resolve_shouldShowNux(self, graphene_info):
return graphene_info.context.instance.nux_enabled and not get_has_seen_nux()
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
from dagster_graphql.test.utils import (
execute_dagster_graphql,
)

SET_NUX_SEEN_MUTATION = """
mutation SetNuxSeen {
setNuxSeen
}
"""

GET_SHOULD_SHOW_NUX_QUERY = """
query ShouldShowNux {
shouldShowNux
}
"""


def test_stores_nux_seen_state(graphql_context):
result = execute_dagster_graphql(graphql_context, GET_SHOULD_SHOW_NUX_QUERY)
assert not result.errors
assert result.data
assert result.data["shouldShowNux"] is True

execute_dagster_graphql(graphql_context, SET_NUX_SEEN_MUTATION)

result = execute_dagster_graphql(graphql_context, GET_SHOULD_SHOW_NUX_QUERY)
assert not result.errors
assert result.data
assert result.data["shouldShowNux"] is False
16 changes: 16 additions & 0 deletions python_modules/dagster/dagster/_core/instance/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,22 @@ def telemetry_enabled(self) -> bool:
else:
return dagster_telemetry_enabled_default

@property
def nux_enabled(self) -> bool:
if self.is_ephemeral:
return False

nux_enabled_by_default = True

nux_settings = self.get_settings("nux")
if not nux_settings:
return nux_enabled_by_default

if "enabled" in nux_settings:
return nux_settings["enabled"]
else:
return nux_enabled_by_default

# run monitoring

@property
Expand Down
28 changes: 28 additions & 0 deletions python_modules/dagster/dagster/_core/nux.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import os

import yaml

from dagster._core.telemetry import get_or_create_dir_from_dagster_home

NUX_FILE_STR = "nux.yaml"


# Filepath for where we store if we've seen the nux
def nux_seen_filepath():
return os.path.join(get_or_create_dir_from_dagster_home(".nux"), "nux.yaml")


# Sets whether we've shown the Nux to any user on this instance
def set_nux_seen():
try: # In case we encounter an error while writing to user's file system
with open(nux_seen_filepath(), "w", encoding="utf8") as nux_seen_file:
# the contents of the file dont matter, we just check that it exists, but lets write seen: True here anyways.
yaml.dump({"seen": 1}, nux_seen_file, default_flow_style=False)
except Exception:
return "<<unable_to_write_nux_seen>>"


# Gets whether we've shown the Nux to any user on this instance
def get_has_seen_nux():
# We only care about the existence of the file
return os.path.exists(nux_seen_filepath())

0 comments on commit 1195798

Please sign in to comment.