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 token only if not present, also to fileURL parameters #214

Merged
merged 14 commits into from
Dec 2, 2024

Conversation

burnout87
Copy link
Contributor

No description provided.

@burnout87
Copy link
Contributor Author

burnout87 commented Nov 19, 2024

As described here oda-hub/dispatcher-app#719 (comment) , the token will be added in the following cases:

  • the parameter is a FileReference, a POSIXPath or a FileURL
  • the parameter is a valid URL
  • the parameter is also an mmoda URL

Eventually, the parameter will be downloaded only if this is a POSIXPath, whereas in the case of FileReference or FileURL this will be "downloadable"

@burnout87 burnout87 changed the title add token only if not present add token only if not present, also to fileURL parameters Nov 19, 2024
@burnout87 burnout87 requested a review from dsavchenko November 19, 2024 14:50
@burnout87
Copy link
Contributor Author

can you please merge @volodymyrss ?

@dsavchenko
Copy link
Member

Don't merge it. We are discussing additional cases

@burnout87 burnout87 marked this pull request as draft November 22, 2024 10:16
@burnout87
Copy link
Contributor Author

Now it is additionally checking the FileURL type, and issuing a ValueError exception in case a not-url arg is passed in the query

@burnout87 burnout87 marked this pull request as ready for review November 25, 2024 12:34
@burnout87
Copy link
Contributor Author

It seems like the issue discussed in #210 is happening also here https://github.com/oda-hub/nb2workflow/actions/runs/12010102302/job/33505494557

Should we merge anyway?

@burnout87
Copy link
Contributor Author

can you please merge @volodymyrss ?

Now @volodymyrss can merge

@dsavchenko dsavchenko enabled auto-merge (squash) December 2, 2024 16:52
@dsavchenko dsavchenko merged commit 91e0a55 into master Dec 2, 2024
2 checks passed
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.

if FileReference is used as an input, and the file is uploaded, the URL is not accessible
3 participants