Skip to content

Conversation

sahu-sunil
Copy link

@sahu-sunil sahu-sunil commented Apr 1, 2025

What does this PR do?

User provided aws-bucket-key-prefix-source and aws-bucket-key-prefix-destination were not respected instead default paths were taken. This MR fixes the issue.

Description of the Change

While creating the State object we have evaluated the correct source and destination paths here, and trying to get from their envvar names in State which looks incorrect

Alternate Designs

Possible Drawbacks

Verification Process

Additional Notes

Release Notes

Review checklist (to be filled by reviewers)

  • Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • PR should not have do-not-merge/ label attached.
  • If Applicable, issue must have kind/ and severity/ labels attached at least.

@sahu-sunil sahu-sunil requested a review from a team as a code owner April 1, 2025 14:55
@sahu-sunil
Copy link
Author

@michael-richey Could you please review this?

@michael-richey
Copy link
Collaborator

User provided aws-bucket-key-prefix-source and aws-bucket-key-prefix-destination were not respected

Can you explain this a little further? I've run things like the following without issue:

datadog-sync migrate --resources="dashboards" --storage-type=s3 --aws-bucket-name=test --aws-bucket-key-prefix-source=myprefix/source  --aws-bucket-key-prefix-destination=myprefix/destination --aws-region-name=us-east-1 

If I can replicate what you're seeing then we might be able to resolve the issue rather than eliminating the use of the env vars which is what it appears this PR does. Eliminating the env vars might still be the solution, but I'd just like to understand the problem you're facing better.

@sahu-sunil
Copy link
Author

sahu-sunil commented May 29, 2025

@michael-richey
So after running the above command, did you verify what paths it created in S3? Actually it always takes default paths resource/source and resource/destination

It's not getting from env vars instead it's looking those env vars in kwargs, which isn't present

@michael-richey
Copy link
Collaborator

Ah! You're right and I see how this fixes it. Thank you.

@michael-richey
Copy link
Collaborator

michael-richey commented May 29, 2025

Merged your change in here: #349
I guessed it would be easier to do that than troubleshoot the tests here. Thanks for the fix!

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.

2 participants