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 upload_entities params #278

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

ptinsl
Copy link

@ptinsl ptinsl commented Jan 10, 2024

forces any managed attributes used in the bulkEntities sheet to use the replace behaviour by default.
I think replace should be the default behaviour. Leaving cell blank will clear any previous inputs.

Previously, no action was being taken with Managed Attributes

@wjohnson
Copy link
Owner

@ptinsl this looks like a great and helpful change. Thank you!

Would you be open to the following changes to make it more consistent with the package?

  1. This behavior is only for the Purview endpoints, so we'd need to make this method in the PurviewClient and make the super function in AtlasClient be able to accept arbitrary parameters.
    • I would suggest adding a parameters={} to the argument list for AtlasClient.upload_entities and then just passing params=parameters to the HTTP calls.
  2. For the PurviewClient you'd need to add upload_entities and add a parameter businessAttributeUpdateBehavior (with a default argument of None) to the upload_entity method after batch_size=None.
  3. PurviewClient.upload_entities should then test if businessAttributeUpdateBehavior is set and then pass the key (businessAttributeUpdateBehavior) and value to the AtlasClient.upload_entities(... parameters={...}) method.
  4. Document that parameter and the allowed values (even better if we link ) in the docstring for PurviewClient.upload_entities.

If you'd be open to making these changes, I'd be glad to accept your PR! Thank you for taking the time to contribute and take this feedback.

@ptinsl
Copy link
Author

ptinsl commented Jan 16, 2024

Hi @wjohnson. I have made the changes you suggested. I have also included collectionId as this is also new.

Not sure if this would be the preferred endpoint for using this functionality or if it deprecates the need for PurviewCollectionsClient.upload_entities()

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