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 import to random_pet resource #184 #257

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

Conversation

wakeful
Copy link
Contributor

@wakeful wakeful commented May 23, 2022

No description provided.

@wakeful wakeful requested a review from a team as a code owner May 23, 2022 20:16
@wakeful wakeful force-pushed the add-import-support-to-random-pet-resource branch from 6ab188b to 58fd7f2 Compare May 23, 2022 20:25
@wakeful wakeful force-pushed the add-import-support-to-random-pet-resource branch from 58fd7f2 to 5c9c363 Compare June 8, 2022 13:16
@github-actions github-actions bot added size/S and removed size/XS labels Jun 8, 2022
@MikaelFerland
Copy link

@bendbennett, do you know if anyone that may help with that ?

@wakeful
Copy link
Contributor Author

wakeful commented Oct 21, 2022

gosh.. I forgot about this MR,
I will rebase & port my fix to work with latest main later today.

@wakeful wakeful force-pushed the add-import-support-to-random-pet-resource branch from 5c9c363 to 31922e3 Compare October 22, 2022 12:02
@wakeful
Copy link
Contributor Author

wakeful commented Oct 22, 2022

I ported my change to the latest version of main
one improvement over the previous version - import now supports adding a custom prefix and separator

@MikaelFerland can you check by building from my branch? if the change works for you? is anything missing?

@MikaelFerland
Copy link

@wakeful I could try, but after I will not able to do anything. I'm not working for HashiCorp.
@bendbennett do we have to do anything else to have it reviewed ?

@mjperrone
Copy link

I'd like this merged

@bendbennett
Copy link
Contributor

Hi @wakeful, @MikaelFerland, @mjperrone 👋

Thank you for submitting the PR @wakeful and apologies in the delay in us getting to it.

Your proposal is an interesting one, as you're essentially overloading the import id with additional attributes/fields for use in populating the resource during import. There are a number of use cases described for this scenario.

At this time, we are intending to implement Import via Configuration for providers developed with the Framework to deliver the ability to import multiple attributes/fields. We would therefore like to hold off and avoid "overloading" the import id until Import via Configuration becomes available.

@wakeful
Copy link
Contributor Author

wakeful commented Nov 9, 2022

hi @bendbennett that's an interesting proposal, it would simplify a lot!

my change came from a migration project that I'm currently working on,
but I don't mind running a custom provider build.

let's park this change till you guys implement import via configuration.

@jhyelton
Copy link

jhyelton commented Mar 6, 2024

Any update to this issue now that import via configuration has been implemented?

@wakeful
Copy link
Contributor Author

wakeful commented Mar 6, 2024

hey @jhyelton

TBH I forgot already about this PR 🤦‍♂️
added to ToDo for this week to do a rebase and check what's needed :)

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

Successfully merging this pull request may close these issues.

None yet

5 participants