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

🎉 Source Pinterest: filters for entity statuses #15074

Merged
merged 61 commits into from
Nov 8, 2022

Conversation

drewrasm
Copy link
Contributor

@drewrasm drewrasm commented Jul 27, 2022

What

Previously there was no way to filter off of entity_statuses in a Pinterest integration. Here are the docs for the url params.

How

In the source for Pinterest, the option to allow a status for the entities that can be filtered off of statuses was added.

Screen Shot 2022-07-26 at 10 12 32 AM

Recommended reading order

  1. spec.json
  2. source.py

🚨 User Impact 🚨

The parameters are totally optional, so there is no impact on current functionality.

Pre-merge Checklist

Updating a connector

Community member or Airbyter

  • Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub and connector version bumped by running the /publish command described here

Tests

Unit
Building Airbyte Sub Build: PLATFORM
Type-safe dependency accessors is an incubating feature.

> Task :airbyte-config:specs:generateSeedConnectorSpecs
2022-07-27 16:34:51 INFO i.a.c.EnvConfigs(getEnvOrDefault):967 - Using default value for environment variable SPEC_CACHE_BUCKET: 'io-airbyte-cloud-spec-cache'
2022-07-27 16:34:51 INFO i.a.c.s.SeedConnectorSpecGenerator(run):78 - Updating seeded SOURCE definition specs if necessary...
2022-07-27 16:34:51 INFO i.a.c.s.SeedConnectorSpecGenerator(run):90 - Finished updating /Users/Andrew/projects/airbyte/airbyte-config/init/src/main/resources/seed/source_specs.yaml
2022-07-27 16:34:51 INFO i.a.c.s.SeedConnectorSpecGenerator(run):78 - Updating seeded DESTINATION definition specs if necessary...
2022-07-27 16:34:51 INFO i.a.c.s.SeedConnectorSpecGenerator(run):90 - Finished updating /Users/Andrew/projects/airbyte/airbyte-config/init/src/main/resources/seed/destination_specs.yaml

> Task :airbyte-tests:pmdAcceptanceTests
Execution optimizations have been disabled for task ':airbyte-tests:pmdAcceptanceTests' to ensure correctness due to the following reasons:
  - Gradle detected a problem with the following location: '/Users/Andrew/projects/airbyte/airbyte-tests/build/resources/acceptanceTests'. Reason: Task ':airbyte-tests:pmdAcceptanceTests' uses this output of task ':airbyte-tests:copyComposeFileForAcceptanceTests' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.4/userguide/validation_problems.html#implicit_dependency for more details about this problem.

> Task :airbyte-tests:pmdAutomaticMigrationAcceptanceTest
Execution optimizations have been disabled for task ':airbyte-tests:pmdAutomaticMigrationAcceptanceTest' to ensure correctness due to the following reasons:
  - Gradle detected a problem with the following location: '/Users/Andrew/projects/airbyte/airbyte-tests/build/resources/automaticMigrationAcceptanceTest'. Reason: Task ':airbyte-tests:pmdAutomaticMigrationAcceptanceTest' uses this output of task ':airbyte-tests:copyComposeFileForMigrationAcceptanceTests' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.4/userguide/validation_problems.html#implicit_dependency for more details about this problem.

> Task :airbyte-webapp:validateLinks

> airbyte-webapp@0.39.37-alpha validate-links
> ts-node --skip-project ./scripts/validate-links.ts

✓ [tutorialLink] https://www.youtube.com/watch?v=Rcpt5SVsMpk&feature=emb_logo returned HTTP 200
✓ [docsLink] https://docs.airbyte.com returned HTTP 200
✓ [configurationArchiveLink] https://docs.airbyte.com/operator-guides/upgrading-airbyte/ returned HTTP 200
✓ [supportTicketLink] https://airbyte.com/contact-support returned HTTP 200
✓ [helpLink] https://airbyte.com/community returned HTTP 200
✓ [privacyLink] https://airbyte.com/privacy-policy returned HTTP 200
✓ [termsLink] https://airbyte.com/terms returned HTTP 200
✓ [updateLink] https://docs.airbyte.com/operator-guides/upgrading-airbyte returned HTTP 200
✓ [webpageLink] https://airbyte.com returned HTTP 200
✓ [productReleaseStages] https://docs.airbyte.com/project-overview/product-release-stages returned HTTP 200
✓ [statusLink] https://status.airbyte.io/ returned HTTP 200
✓ [normalizationLink] https://docs.airbyte.com/understanding-airbyte/connections#airbyte-basic-normalization returned HTTP 200
✓ [syncModeLink] https://docs.airbyte.com/understanding-airbyte/connections returned HTTP 200
✓ [contactSales] https://airbyte.com/talk-to-sales returned HTTP 200
✓ [namespaceLink] https://docs.airbyte.com/understanding-airbyte/namespaces returned HTTP 200
✓ [gitLink] https://docs.airbyte.com/quickstart/deploy-airbyte returned HTTP 200
✓ [technicalSupport] https://docs.airbyte.com/troubleshooting/on-deploying returned HTTP 200
✓ [demoLink] https://demo.airbyte.io returned HTTP 200
✓ [slackLink] https://slack.airbyte.com returned HTTP 200
✓ [recipesLink] https://airbyte.com/recipes returned HTTP 200

✓ All URLs have been checked successfully.

> Task :airbyte-webapp:test

> airbyte-webapp@0.39.37-alpha pretest
> npm run generate-client


> airbyte-webapp@0.39.37-alpha generate-client
> orval

🍻 Start orval v6.8.1 - A swagger client generator for typescript
🎉 api - Your OpenAPI spec has been converted into ready to use orval!

> airbyte-webapp@0.39.37-alpha test
> craco test "--watchAll=false" "--silent"

PASS src/hooks/services/Feature/FeatureService.test.tsx
PASS src/core/i18n/I18nProvider.test.tsx
PASS src/utils/useTranslateDataType.test.tsx
PASS src/core/jsonSchema/schemaToYup.test.ts
PASS src/hooks/services/Experiment/ExperimentService.test.tsx
PASS src/core/form/uiWidget.test.ts
PASS src/views/Connection/ConnectionForm/calculateInitialCatalog.test.ts
PASS src/core/jsonSchema/schemaToUiWidget.test.ts
PASS src/core/jsonSchema/utils.test.ts
PASS src/hooks/useTrackAction.test.tsx
PASS src/utils/common.test.ts
PASS src/utils/utmStorage.test.ts
PASS src/config/configProviders.test.ts
PASS src/components/StatusIcon/StatusIcon.test.tsx
PASS src/components/ArrayOfObjectsEditor/components/EditorHeader.test.tsx
PASS src/packages/cloud/views/auth/components/GitBlock/GitBlock.test.tsx
PASS src/components/base/Input/Input.test.tsx
PASS src/hooks/services/Modal/ModalService.test.tsx
PASS src/views/Connection/ConnectionForm/ConnectionForm.test.tsx
PASS src/views/Connector/ServiceForm/ServiceForm.test.tsx (7.622 s)

Test Suites: 1 skipped, 20 passed, 20 of 21 total
Tests:       2 skipped, 121 passed, 123 total
Snapshots:   16 passed, 16 total
Time:        8.508 s, estimated 21 s

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.4/userguide/command_line_interface.html#sec:command_line_warnings

Execution optimizations have been disabled for 2 invalid unit(s) of work during this build to ensure correctness.
Please consult deprecation warnings for more details.

BUILD SUCCESSFUL in 22s
373 actionable tasks: 5 executed, 368 up-to-date
Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

@CLAassistant
Copy link

CLAassistant commented Jul 27, 2022

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
20 out of 21 committers have signed the CLA.

✅ drewrasm
✅ marcosmarxm
✅ xiaohansong
✅ cpdeethree
✅ tuliren
✅ grishick
✅ artem1205
✅ perangel
✅ jdpgrailsdev
✅ lazebnyi
✅ darynaishchenko
✅ josephkmh
✅ cgardens
✅ YowanR
✅ subodh1810
✅ timroes
✅ cuyk
✅ TBernstein4
✅ mfsiega-airbyte
✅ Amruta-Ranade
❌ github-actions[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

@drewrasm drewrasm changed the title filters for statuses 🎉 Pinterest filters for statuses Jul 27, 2022
@marcosmarxm marcosmarxm changed the title 🎉 Pinterest filters for statuses 🎉 Source Pinterest: filters for entity statuses Aug 1, 2022
@marcosmarxm
Copy link
Member

@drewrasm please sign the CLA.

@marcosmarxm
Copy link
Member

@misteryeo can you check the change in the connector specification?

@sajarin sajarin added the bounty-S Maintainer program: claimable small bounty PR label Aug 5, 2022
@drewrasm
Copy link
Contributor Author

drewrasm commented Aug 8, 2022

@marcosmarxm let me know if there's anything in your requested changes I need to address, thanks! 👍

@marcosmarxm
Copy link
Member

marcosmarxm commented Aug 8, 2022

/test connector=connectors/source-pinterest

🕑 connectors/source-pinterest https://github.com/airbytehq/airbyte/actions/runs/2820267854
❌ connectors/source-pinterest https://github.com/airbytehq/airbyte/actions/runs/2820267854
🐛 https://gradle.com/s/ea4b3r3vfd6ea

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - Failed: Please check...
FAILED test_incremental.py::TestIncremental::test_read_sequential_slices[inputs0]
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_incremental.py:23: `future_state_path` not specified, skipping
======== 2 failed, 25 passed, 1 skipped, 1 warning in 459.05s (0:07:39) ========

@drewrasm
Copy link
Contributor Author

/test connector=connectors/source-pinterest

@drewrasm
Copy link
Contributor Author

@marcosmarxm I had the same failures in the tests happening independent if I was on my branch or master locally. I fixed what I could locally of the tests that were broken. Can you verify my changes broke the tests and that they weren't already failing? That would help out tremendously, thanks!

@marcosmarxm
Copy link
Member

I think our account is not passing the tests, I need to investigate further but asap I discovered that I'll return to you. Thanks for the contribution

@sajarin sajarin added internal and removed bounty bounty-S Maintainer program: claimable small bounty PR labels Aug 11, 2022
@drewrasm
Copy link
Contributor Author

@marcosmarxm thanks for checking into that, is there any update towards getting those tests resolved? Please don't hesitate to let me know of anything I'll need to change in the PR. Thanks again!

@drewrasm
Copy link
Contributor Author

@misteryeo would you be able to give us an update on the progress of getting the tests resolved? Thank you for your time!

@wgormley
@coltonpomeroy

@misteryeo
Copy link
Contributor

Apologies for the delay here! Checking in with @marcosmarxm on this as he's the one investigating and will come back to you with an update!

@marcosmarxm
Copy link
Member

Sorry delay here @drewrasm I'll take a look tomorrow!

@coltonpomeroy
Copy link

Hey @marcosmarxm, just wanted to check-in and see if you were able to take a look?

@marcosmarxm
Copy link
Member

Yes @coltonpomeroy but I'm having problem with the API token access. Hope get this solved today/tomorrow. Sorry the delay to fix it and merge your contribution

@coltonpomeroy
Copy link

Hey @marcosmarxm, were you able to resolve the issue you were having with the API token access?

@github-actions github-actions bot removed area/frontend Related to the Airbyte webapp area/server area/platform issues related to the platform area/worker Related to worker kubernetes normalization labels Nov 2, 2022
@drewrasm
Copy link
Contributor Author

drewrasm commented Nov 2, 2022

@arsenlosenko
I changed the dockerfile and pulled from most recent master. It seems to have automatically done some rebasing, which added all those new labels.
Can we get this merged today with the change to the dockerfile?

@arsenlosenko
Copy link
Contributor

arsenlosenko commented Nov 3, 2022

/publish connector=connectors/source-pinterest

🕑 Publishing the following connectors:
connectors/source-pinterest
https://github.com/airbytehq/airbyte/actions/runs/3385378044


Connector Did it publish? Were definitions generated?
connectors/source-pinterest

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@arsenlosenko
Copy link
Contributor

@drewrasm thanks for added changes and keeping this PR up to date compared to master branch, I am publishing this version of connector now, once it is all done, the PR will be merged.

@drewrasm
Copy link
Contributor Author

drewrasm commented Nov 3, 2022

@arsenlosenko thank you!
I see that the publish command was run, is there anything you need from us?

@arsenlosenko
Copy link
Contributor

arsenlosenko commented Nov 7, 2022

/publish connector=connectors/source-pinterest run-tests=false

🕑 Publishing the following connectors:
connectors/source-pinterest
https://github.com/airbytehq/airbyte/actions/runs/3408716942


Connector Did it publish? Were definitions generated?
connectors/source-pinterest

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@arsenlosenko
Copy link
Contributor

@drewrasm the previous command failed because of the tests, skipping them now and publishing again

@arsenlosenko
Copy link
Contributor

@drewrasm the changes were published, will merge it once the required checks have passed

@arsenlosenko
Copy link
Contributor

@marcosmarxm I see that you have requested changes in this PR earlier, the PR is ready to be merged, can you please take a look if your requested changes were done?

@drewrasm
Copy link
Contributor Author

drewrasm commented Nov 7, 2022

@arsenlosenko Awesome, that's great news! Those requested changes should be taken care of. I have comments about how I resolved those requests.
Is there a way you can override the requested changes from @marcosmarxm? He hasn't responded to my re-requested review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment