Skip to content

feat(onboarding): add poetry as an option to python install snippets #90365

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

Merged
Merged
2 changes: 1 addition & 1 deletion static/app/gettingStartedDocs/python/bottle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const onboarding: OnboardingConfig = {
code: <code />,
}
),
configurations: getPythonInstallConfig({packageName: "'sentry-sdk[bottle]'"}),
configurations: getPythonInstallConfig({packageName: 'sentry-sdk[bottle]'}),
},
],
configure: (params: Params) => [
Expand Down
2 changes: 1 addition & 1 deletion static/app/gettingStartedDocs/python/celery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const onboarding: OnboardingConfig = {
code: <code />,
}
),
configurations: getPythonInstallConfig({packageName: "'sentry-sdk[celery]'"}),
configurations: getPythonInstallConfig({packageName: 'sentry-sdk[celery]'}),
},
],
configure: (params: Params) => [
Expand Down
2 changes: 1 addition & 1 deletion static/app/gettingStartedDocs/python/chalice.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const onboarding: OnboardingConfig = {
code: <code />,
}
),
configurations: getPythonInstallConfig({packageName: "'sentry-sdk[chalice]'"}),
configurations: getPythonInstallConfig({packageName: 'sentry-sdk[chalice]'}),
},
],
configure: (params: Params) => [
Expand Down
2 changes: 1 addition & 1 deletion static/app/gettingStartedDocs/python/django.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const onboarding: OnboardingConfig = {
code: <code />,
}
),
configurations: getPythonInstallConfig({packageName: "'sentry-sdk[django]'"}),
configurations: getPythonInstallConfig({packageName: 'sentry-sdk[django]'}),
},
],
configure: (params: Params) => [
Expand Down
2 changes: 1 addition & 1 deletion static/app/gettingStartedDocs/python/fastapi.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ const onboarding: OnboardingConfig = {
code: <code />,
}
),
configurations: getPythonInstallConfig({packageName: "'sentry-sdk[fastapi]'"}),
configurations: getPythonInstallConfig({packageName: 'sentry-sdk[fastapi]'}),
},
],
configure: (params: Params) => [
Expand Down
2 changes: 1 addition & 1 deletion static/app/gettingStartedDocs/python/flask.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ const onboarding: OnboardingConfig = {
code: <code />,
}
),
configurations: getPythonInstallConfig({packageName: "'sentry-sdk[flask]'"}),
configurations: getPythonInstallConfig({packageName: 'sentry-sdk[flask]'}),
},
],
configure: (params: Params) => [
Expand Down
2 changes: 1 addition & 1 deletion static/app/gettingStartedDocs/python/mongo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const onboarding: OnboardingConfig = {
code: <code />,
}
),
configurations: getPythonInstallConfig({packageName: "'sentry-sdk[pymongo]'"}),
configurations: getPythonInstallConfig({packageName: 'sentry-sdk[pymongo]'}),
},
],
configure: (params: Params) => [
Expand Down
2 changes: 1 addition & 1 deletion static/app/gettingStartedDocs/python/quart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ const onboarding: OnboardingConfig = {
code: <code />,
}
),
configurations: getPythonInstallConfig({packageName: "'sentry-sdk[quart]'"}),
configurations: getPythonInstallConfig({packageName: 'sentry-sdk[quart]'}),
},
],
configure: (params: Params) => [
Expand Down
2 changes: 1 addition & 1 deletion static/app/gettingStartedDocs/python/rq.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ const onboarding: OnboardingConfig = {
description: tct('Install [code:sentry-sdk] from PyPI with the [code:rq] extra:', {
code: <code />,
}),
configurations: getPythonInstallConfig({packageName: "'sentry-sdk[rq]'"}),
configurations: getPythonInstallConfig({packageName: 'sentry-sdk[rq]'}),
},
],
configure: (params: Params) => [
Expand Down
2 changes: 1 addition & 1 deletion static/app/gettingStartedDocs/python/sanic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const onboarding: OnboardingConfig = {
}
),
configurations: [
...getPythonInstallConfig({packageName: "'sentry-sdk[sanic]'"}),
...getPythonInstallConfig({packageName: 'sentry-sdk[sanic]'}),
...getPythonAiocontextvarsConfig(),
],
},
Expand Down
2 changes: 1 addition & 1 deletion static/app/gettingStartedDocs/python/starlette.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ const onboarding: OnboardingConfig = {
code: <code />,
}
),
configurations: getPythonInstallConfig({packageName: "'sentry-sdk[starlette]'"}),
configurations: getPythonInstallConfig({packageName: 'sentry-sdk[starlette]'}),
},
],
configure: (params: Params) => [
Expand Down
63 changes: 63 additions & 0 deletions static/app/utils/gettingStartedDocs/python.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Mock the modules that cause circular dependencies
jest.mock('sentry/utils/gettingStartedDocs/python', () => {
const original = jest.requireActual('sentry/utils/gettingStartedDocs/python');
return {
...original,
// Mock any functions causing circular dependencies
getPythonProfilingOnboarding: jest.fn(),
};
});
Comment on lines +1 to +9
Copy link
Member

Choose a reason for hiding this comment

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

is this needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

We do need it, because otherwise the tests don't work


import {getPythonInstallSnippet} from 'sentry/utils/gettingStartedDocs/python';

describe('getPythonInstallSnippet', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we maybe write those tests against getPythonInstallConfig instead of getPythonInstallSnippet?

getPythonInstallSnippet is only used inside getPythonInstallConfig, so it’s an implementation detail of getPythonInstallSnippet. The “public interface” that is used by consumers is getPythonInstallConfig, and we should be free to change how that is implemented internally without having to change tests

Copy link
Member Author

Choose a reason for hiding this comment

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

Will be resolved in #91119

it('generates pip install command with default parameters', () => {
const result = getPythonInstallSnippet({
packageName: 'sentry-sdk',
});

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

expect(result.pip).toBe(`pip install "sentry-sdk"`);
});

it('generates pip install command with minimum version and extras', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
it('generates pip install command with minimum version and extras', () => {
it('generates install commands with minimum version and extras', () => {

now we test more than just pip :)

const result = getPythonInstallSnippet({
packageName: 'sentry-sdk[with-extras]',
minimumVersion: '1.2.3',
});
expect(result.pip).toBe(`pip install --upgrade "sentry-sdk[with-extras]>=1.2.3"`);
});

it('generates uv install command with default parameters', () => {
const result = getPythonInstallSnippet({
packageName: 'sentry-sdk',
});

expect(result.uv).toBe(`uv add "sentry-sdk"`);
});
Copy link
Member

Choose a reason for hiding this comment

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

we could combine the tests for uv, pip, and poetry into a single one - this test , for example, could be combined with the first one

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point - changed


it('generates uv install command with minimum version and extras', () => {
const result = getPythonInstallSnippet({
packageName: 'sentry-sdk[with-extras]',
minimumVersion: '2.3.4',
});

expect(result.uv).toBe(`uv add --upgrade "sentry-sdk[with-extras]>=2.3.4"`);
});

it('generates poetry install command with default parameters', () => {
const result = getPythonInstallSnippet({
packageName: 'sentry-sdk',
});
Copy link
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
const result = getPythonInstallSnippet({
packageName: 'sentry-sdk',
});
const result = getPythonInstallSnippet();

Isn't "sentry-sdk" the default packageName?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not in the InstallSnippet; only in the InstallConfig, which is why it needs to be passed in here in the test


expect(result.poetry).toBe(`poetry add "sentry-sdk"`);
});

it('generates poetry install command with minimum version and extras', () => {
const result = getPythonInstallSnippet({
packageName: 'sentry-sdk[with-extras]',
minimumVersion: '2.3.4',
});

expect(result.poetry).toBe(`poetry add "sentry-sdk[with-extras]>=2.3.4"`);
});
});
34 changes: 17 additions & 17 deletions static/app/utils/gettingStartedDocs/python.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,36 +12,38 @@ import {t, tct} from 'sentry/locale';

export function getPythonInstallSnippet({
packageName,
packageManager = 'pip',
minimumVersion,
}: {
packageName: string;
minimumVersion?: string;
packageManager?: 'pip' | 'uv';
}) {
// We are using consistent double quotes here for all package managers after aligning with the Python SDK team.
// Not using quotes may lead to some shells interpreting the square brackets, and using double quotes over single quotes is a convention.
const versionedPackage = minimumVersion
? `${packageName}>=${minimumVersion}`
: packageName;
? `"${packageName}>=${minimumVersion}"`
: `"${packageName}"`;

const upgradeFlag = minimumVersion ? '--upgrade ' : '';

const packageManagerCommands = {
uv: `uv add ${upgradeFlag}${versionedPackage}`,
pip: `pip install ${upgradeFlag}${versionedPackage}`,
poetry: `poetry add ${versionedPackage}`,
};

return packageManagerCommands[packageManager].trim();
return packageManagerCommands;
}

export function getPythonInstallConfig({
packageName = "'sentry-sdk'",
packageName = 'sentry-sdk',
description,
minimumVersion,
}: {
description?: React.ReactNode;
minimumVersion?: string;
packageName?: string;
} = {}): Configuration[] {
const packageManagerCommands = getPythonInstallSnippet({packageName, minimumVersion});
return [
{
description,
Expand All @@ -51,21 +53,19 @@ export function getPythonInstallConfig({
label: 'pip',
value: 'pip',
language: 'bash',
code: getPythonInstallSnippet({
packageName,
packageManager: 'pip',
minimumVersion,
}),
code: packageManagerCommands.pip,
},
{
label: 'uv',
value: 'uv',
language: 'bash',
code: getPythonInstallSnippet({
packageName,
packageManager: 'uv',
minimumVersion,
}),
code: packageManagerCommands.uv,
},
{
Copy link
Member

Choose a reason for hiding this comment

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

since all of the 3x packages are being used here. what about just return

return {
    uv: `uv add ${upgradeFlag}${versionedPackage}`,
    pip: `pip install ${upgradeFlag}${versionedPackage}`,
    poetry: `poetry add ${versionedPackage}`,
  }

in the getPythonInstallSnippet? this way we don't need to call the function 3x times but only 1x and we can do pythonInstallSnippet.poetry for example

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, changed it 👍

label: 'poetry',
value: 'poetry',
language: 'bash',
code: packageManagerCommands.poetry,
},
],
},
Expand Down Expand Up @@ -93,7 +93,7 @@ export function getPythonAiocontextvarsConfig({
);

return getPythonInstallConfig({
packageName: "'aiocontextvars'",
packageName: 'aiocontextvars',
description: description ?? defaultDescription,
});
}
Expand Down
Loading