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.

Sign up for GitHub

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

Override Notion Client logger #2620

Merged
merged 3 commits into from
Nov 22, 2023
Merged

Override Notion Client logger #2620

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
880109c
Override Notion Client logger
PopDaph Nov 22, 2023
ccbc6a8
Do not even log
PopDaph Nov 22, 2023
40669e8
Change my mind again
PopDaph Nov 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 49 additions & 10 deletions connectors/src/connectors/notion/lib/notion_api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
isFullBlock,
isFullDatabase,
isFullPage,
LogLevel,
} from "@notionhq/client";
import {
BlockObjectResponse,
Expand All @@ -30,6 +31,14 @@ import mainLogger from "@connectors/logger/logger";

const logger = mainLogger.child({ provider: "notion" });

const notionClientLogger = (
level: LogLevel,
message: string,
extraInfo: Record<string, unknown>
) => {
console.log(`[${level}]: ${message}`, extraInfo);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not directly use a logger here? potentially deactivating after that?

Copy link
Contributor

Choose a reason for hiding this comment

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

We def don't want those logs to clutter our DD errors no ? The'll handle the requests to notion, and we'll do the logging if where we need it wdyt ?

Copy link
Contributor

@spolu spolu Nov 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not too worried about cluttering. But my point here is that console.log will clutter more than logger ?

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 removed the console.log and no-op instead > want me to add back a log in info?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK added back with a logger.info so we keep them but stop being spammed with errors

};

/**
* @param notionAccessToken the access token to use to access the Notion API
* @param sinceTs a millisecond timestamp representing the minimum last edited time of
Expand Down Expand Up @@ -58,7 +67,10 @@ export async function getPagesAndDatabasesEditedSince(
}> {
const localLogger = logger.child(loggerArgs);

const notionClient = new Client({ auth: notionAccessToken });
const notionClient = new Client({
auth: notionAccessToken,
logger: notionClientLogger,
});
const editedPages: Record<string, number> = {};
const editedDbs: Record<string, number> = {};
let resultsPage: SearchResponse | null = null;
Expand Down Expand Up @@ -217,7 +229,10 @@ export async function getDatabaseChildPages({
}> {
const localLogger = logger.child(loggerArgs);

const notionClient = new Client({ auth: notionAccessToken });
const notionClient = new Client({
auth: notionAccessToken,
logger: notionClientLogger,
});
let resultsPage: QueryDatabaseResponse | null = null;
const pages: Record<string, number> = {};

Expand Down Expand Up @@ -294,7 +309,10 @@ export async function isAccessibleAndUnarchived(
objectType: "page" | "database",
localLogger?: Logger
): Promise<boolean> {
const notionClient = new Client({ auth: notionAccessToken });
const notionClient = new Client({
auth: notionAccessToken,
logger: notionClientLogger,
});
const maxTries = 5;
let tries = 0;

Expand Down Expand Up @@ -373,7 +391,10 @@ async function getBlockParent(
const max_depth = 8;
const max_transient_errors = 5;

const notionClient = new Client({ auth: notionAccessToken });
const notionClient = new Client({
auth: notionAccessToken,
logger: notionClientLogger,
});
let depth = 0;
let transient_errors = 0;

Expand Down Expand Up @@ -442,7 +463,10 @@ export async function getParsedDatabase(
): Promise<ParsedNotionDatabase | null> {
const localLogger = logger.child({ ...loggerArgs, databaseId });

const notionClient = new Client({ auth: notionAccessToken });
const notionClient = new Client({
auth: notionAccessToken,
logger: notionClientLogger,
});

let database: GetDatabaseResponse | null = null;

Expand Down Expand Up @@ -533,7 +557,10 @@ export async function retrievePage({
}): Promise<PageObjectResponse | null> {
const localLogger = logger.child({ ...loggerArgs, pageId });

const notionClient = new Client({ auth: accessToken });
const notionClient = new Client({
auth: accessToken,
logger: notionClientLogger,
});

let page: GetPageResponse | null = null;
try {
Expand Down Expand Up @@ -582,7 +609,10 @@ export async function retrieveBlockChildrenResultPage({
}): Promise<ListBlockChildrenResponse | null> {
const localLogger = logger.child(loggerArgs);

const notionClient = new Client({ auth: accessToken });
const notionClient = new Client({
auth: accessToken,
logger: notionClientLogger,
});

try {
localLogger.info(
Expand Down Expand Up @@ -668,7 +698,10 @@ export function getPageOrBlockParent(
}

export async function validateAccessToken(notionAccessToken: string) {
const notionClient = new Client({ auth: notionAccessToken });
const notionClient = new Client({
auth: notionAccessToken,
logger: notionClientLogger,
});
try {
await notionClient.search({ page_size: 1 });
} catch (e) {
Expand Down Expand Up @@ -768,7 +801,10 @@ export async function retrieveDatabaseChildrenResultPage({
}) {
const localLogger = logger.child({ ...loggerArgs, databaseId });

const notionClient = new Client({ auth: accessToken });
const notionClient = new Client({
auth: accessToken,
logger: notionClientLogger,
});

localLogger.info("Fetching database children result page from Notion API.");
try {
Expand Down Expand Up @@ -848,7 +884,10 @@ export async function getUserName(
return nameFromCache;
}

const notionClient = new Client({ auth: accessToken });
const notionClient = new Client({
auth: accessToken,
logger: notionClientLogger,
});

try {
pageLogger.info({ user_id: userId }, "Fetching user name from Notion API.");
Expand Down