Skip to content

BUG: Blob SAS URL Never Used as Filepath Never None#154

Merged
jordlay merged 1 commit intomainfrom
jl/filepath-never-none-bug
Mar 11, 2024
Merged

BUG: Blob SAS URL Never Used as Filepath Never None#154
jordlay merged 1 commit intomainfrom
jl/filepath-never-none-bug

Conversation

@jordlay
Copy link
Copy Markdown
Collaborator

@jordlay jordlay commented Mar 8, 2024


Because we now make the artifact filepath absolute, it was making empty filepath "" the path to the directory, which means blob sas url was never getting used. Which broke publish.

This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally? (pip install wheel==0.30.0 required)

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify src/index.json.

@jordlay jordlay requested a review from sunnycarter as a code owner March 8, 2024 18:50
@jordlay jordlay changed the title added check for filepath before making absolute BUG: Blob SAS URL Never Used as Filepath Never None Mar 8, 2024
if self.config.vhd.file_path:
file_path = Path(self.config.vhd.file_path).absolute()
else:
file_path = None
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this else statement, do we?

Not a big change, but two lines less is still good :-)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We chatted. Keeping the else

@jordlay jordlay merged commit af1730b into main Mar 11, 2024
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.

2 participants