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

feat: support custom output path #9

Open
pnadolny13 opened this issue Mar 20, 2023 · 3 comments
Open

feat: support custom output path #9

pnadolny13 opened this issue Mar 20, 2023 · 3 comments

Comments

@pnadolny13
Copy link

Related to slack thread https://meltano.slack.com/archives/CMN8HELB0/p1679141123085079

I see that theres several ways to define the naming conventions of the output files but I dont see a way to set a custom output path directory. The person in slack was confused by the output_path_prefix setting because they expected it to allow a custom output path. Ultimately changing variants to use the destination_path with the hotgluexyz variant solved their problem. Either:

  • document how to pass a custom path with the current configurations
  • implement a way to provide a custom path
@amotl
Copy link
Contributor

amotl commented Dec 17, 2023

Hi Pat,

I've just submitted a patch which may improve the situation.

With kind regards,
Andreas.

edgarrmondragon pushed a commit that referenced this issue Dec 18, 2023
## About

Users got confused about the semantics of the `output_path_prefix`
property. This patch makes it better by renaming it to `output_path`,
while still providing backwards-compatibility.

Further, the `destination_path` property can also be used, thus this
target can be a drop-in replacement to the `hotgluexyz` variant.

The value of the variable does not need to be configured using a
trailing slash any longer. Instead, the implementation more thoroughly
leverages `pathlib.Path` for concatenating `output_path` and `filename`.

## References

- GH-9

## Thoughts

I am not sure about fc54324. I added it because mypy tripped like
described in the commit message. Please educate me if that was wrong, so
I will either remove it or amend it correspondingly.
@amotl
Copy link
Contributor

amotl commented Dec 18, 2023

On this topic, thank you for merging GH-50. That was quick. GH-54 adds the missing test cases, which should have accompanied the patch, but were forgotten.

@edgarrmondragon
Copy link
Member

I think we can now close this

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

No branches or pull requests

3 participants