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

feat(api): change calculation variable for pull data from providers #2962

Merged
merged 2 commits into from
Jan 5, 2025

Conversation

00041275
Copy link
Contributor

@00041275 00041275 commented Jan 1, 2025

📑 Description

At the moment, keep prefers to work with webhook from source systems. But my company prefers to initiate a request from keep (for example, Oracle Enterprise Management). In view of this, it is problematic to have a limitation in a variable "KEEP_PULL_INTERVAL", which represents days. As more flexibility, it would be better to give users the ability to configure the pulling time themselves (for example, in minutes). Of course, ideally for the pull data option, it is best to have such a variable in the provider itself, but as an intermediate flexible solution, I suggest considering giving users the freedom to configure data collection from sources themselves.

✅ Checks

  • All the tests have passed

close #2975

Copy link

vercel bot commented Jan 1, 2025

@00041275 is attempting to deploy a commit to the KeepHQ Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. Enhancement New feature or request Provider Providers related issues labels Jan 1, 2025
@talboren
Copy link
Member

talboren commented Jan 1, 2025

Thank you for this PR again @00041275! What do you think if we add a backward compatibility for people who already use this with days? Do you have some idea?
May I also ask you to create an issue for this? 🙏🏼

@talboren
Copy link
Member

talboren commented Jan 1, 2025

Really wish to see you in our Slack too!

@talboren talboren self-requested a review January 2, 2025 13:52
@00041275
Copy link
Contributor Author

00041275 commented Jan 2, 2025

As you can see, I converted 7 days to minutes for backwards compatibility (check this). I didn't find any mention in the documentation part, so I consider this a rarely used feature.

OLD:

PROVIDER_PULL_INTERVAL_DAYS = int(
os.environ.get("KEEP_PULL_INTERVAL", 7)

NEW 7 days * 24 hour * 60 minute = 10 080

PROVIDER_PULL_INTERVAL_MINUTE = int(
os.environ.get("KEEP_PULL_INTERVAL", 10080)

@00041275
Copy link
Contributor Author

00041275 commented Jan 3, 2025

Thank you for this PR again @00041275! What do you think if we add a backward compatibility for people who already use this with days? Do you have some idea? May I also ask you to create an issue for this? 🙏🏼

created #2975

Copy link
Member

@talboren talboren left a comment

Choose a reason for hiding this comment

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

LGTM!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 5, 2025
Copy link

vercel bot commented Jan 5, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
keep ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 5, 2025 10:40am

@talboren talboren changed the title Change calculation variable for pull data from providers feat(api): change calculation variable for pull data from providers Jan 5, 2025
@talboren talboren enabled auto-merge (squash) January 5, 2025 10:29
@talboren talboren merged commit 8ecb6b8 into keephq:main Jan 5, 2025
11 of 12 checks passed
Copy link
Contributor

github-actions bot commented Jan 5, 2025

🎊 You're on fire, @00041275! Three PRs merged and counting! 🔥🎉

Your consistent contributions are truly impressive. You're becoming a valued member of our community! 💖
Have you considered taking on some more challenging issues? We'd love to see what you can do! 💪

Remember, the team is always here to support you. Keep blazing that trail! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request lgtm This PR has been approved by a maintainer Provider Providers related issues size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[➕ Feature]: freedom for variable KEEP_PULL_INTERVAL
2 participants