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

fix(integration): improve guru integration #2835

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hassan254-prog
Copy link
Contributor

Describe your changes

  • This change is necessary to properly handle pagination when processing nextPageLink in Guru. By using the updated base URL, the endpoint construction avoids redundancy.

Issue ticket number and link

Checklist before requesting a review (skip if just adding/editing APIs & templates)

  • I added tests, otherwise the reason is:
  • I added observability, otherwise the reason is:
  • I added analytics, otherwise the reason is:

@@ -1960,7 +1960,7 @@ guru:
- knowledge-base
auth_mode: BASIC
proxy:
base_url: https://api.getguru.com/api/v1
Copy link
Member

Choose a reason for hiding this comment

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

We have to be careful with this for users who are using this in the proxy or scripts. This would be a breaking change that we need to announce etc.

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 agree, we definitely need to handle this carefully to avoid disrupting users who rely on the proxy or scripts. Maybe we can check on the connections currently running in production environment.

Copy link
Collaborator

Choose a reason for hiding this comment

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

there is only 12 existing guru integrations (including 4 in a prod environment) so it will definitely be a breaking change

Copy link
Collaborator

Choose a reason for hiding this comment

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

We could have a second provider, not ideal though
We could introduce a patch props:

guru:
  proxy:
      base_url: https://api.getguru.com/api/v1
  patch
    20241022:
      base_url: https://api.getguru.com/

or the opposite to optimize for new integrations

guru:
  proxy:
      base_url: https://api.getguru.com
  patch
    20241022:
      base_url: https://api.getguru.com/api/v1

So in the code we could use patch depending on the created_at

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants