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

Add Triggers endpoint #81

Merged
merged 10 commits into from
May 9, 2024
Merged

Add Triggers endpoint #81

merged 10 commits into from
May 9, 2024

Conversation

Kukant
Copy link
Contributor

@Kukant Kukant commented May 3, 2024

No description provided.

Copy link
Contributor

@tomasfejfar tomasfejfar left a comment

Choose a reason for hiding this comment

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

This needs intergration test with an actual api.

  • create table
  • create config
  • create trigger on the table to trigger the config
  • run import against the table
  • assert lastRun ofthe trigger

kbcstorage/triggers.py Outdated Show resolved Hide resolved
@Kukant
Copy link
Contributor Author

Kukant commented May 5, 2024

  • run import against the table
  • assert lastRun ofthe trigger

Should I really test the triggering mechanism? That does not seems to be the responsibility of this library.

@tomasfejfar
Copy link
Contributor

Should I really test the triggering mechanism? That does not seems to be the responsibility of this library.

Or you could somehow verify that the trigger is set up correctly.

@Kukant Kukant requested a review from tomasfejfar May 6, 2024 07:35
@odinuv odinuv self-requested a review May 7, 2024 10:27
Copy link
Member

@odinuv odinuv left a comment

Choose a reason for hiding this comment

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

i think it's ok, just please fix the bucket id in test

tests/functional/test_triggers.py Outdated Show resolved Hide resolved
tests/functional/test_triggers.py Outdated Show resolved Hide resolved
tests/functional/test_triggers.py Outdated Show resolved Hide resolved
tests/functional/test_triggers.py Outdated Show resolved Hide resolved
Copy link
Contributor

@tomasfejfar tomasfejfar left a comment

Choose a reason for hiding this comment

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

Odin je s tím OK

@tomasfejfar tomasfejfar merged commit e30f83a into master May 9, 2024
4 of 5 checks passed
@tomasfejfar tomasfejfar deleted the triggers branch May 9, 2024 11:28
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.

3 participants