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

Make temp directory creation more robust, log only on errors #104

Merged
merged 2 commits into from
Jan 18, 2024

Conversation

augustozanellato
Copy link
Contributor

Follow up to PR #103

@fliiiix
Copy link
Contributor

fliiiix commented Oct 2, 2023

what is missing to get this merged and released?

I also saw this 'errors' and looked into it on how to remove them

@fliiiix
Copy link
Contributor

fliiiix commented Oct 29, 2023

@ickc any chance this can make it into a release?

@fliiiix
Copy link
Contributor

fliiiix commented Dec 10, 2023

Any updates on this?

@AlexAltea
Copy link

@jgm Can we get this merged? It's a trivial change, that solves a long standing issue in dependent projects, e.g. timofurrer/pandoc-mermaid-filter#20

@jgm jgm merged commit 4da2c73 into jgm:master Jan 18, 2024
@jgm
Copy link
Owner

jgm commented Jan 18, 2024

I don't use Python and my old notes on how a release is created seem to be obsolete, so I could use help here if you want this officially released to pypi.

@fliiiix
Copy link
Contributor

fliiiix commented Jan 18, 2024

I can help with that i maintain a few python packages like colorful or radish.

what kind of help are you looking for?

@jgm
Copy link
Owner

jgm commented Jan 18, 2024

For now, manual I guess.

I could also use help in fixing the CI failures.

@fliiiix
Copy link
Contributor

fliiiix commented Jan 18, 2024

here is what im doing with radish:

vim CHANGELOG.md docs/conf.py radish/__init__.py
git commit -am "release: vX.X.X" && git tag vX.X.X && git push && git push --tags
rm -rf dist
python3 setup.py sdist bdist_wheel
python3 -m twine upload dist/*

I guess thats very similar to https://github.com/jgm/pandocfilters/blob/master/PACKAGING

main thing that might be confusing is that the twine user ise __token__ and you need to register and api token on pypi https://pypi.org/manage/account/

maybe a python3 -m pip install twine is required if you haven't installed it yet
and setting the release on github is a manual step

@jgm
Copy link
Owner

jgm commented Jan 18, 2024

Interesting, that's what I tried, and I got a mention that using setup.py in this way was deprecated....but I guess it did still create the dist, so maybe I can go forward.

@fliiiix
Copy link
Contributor

fliiiix commented Jan 18, 2024

For pipeline stuff i can take a closer look this weekend some quick observations:

  • Python 3.5 & Python 3.6 fail because they are only supported on the older runners (i used some matrix stuff for that)
  • and the curl grepping for the assets stopped working because assets are now somehow loaded later maybe there is a smart way to do that via the api or something

@jgm
Copy link
Owner

jgm commented Jan 18, 2024

ARNING  Error during upload. Retry with the --verbose option for more details. 
ERROR    HTTPError: 403 Forbidden from https://upload.pypi.org/legacy/          
         Username/Password authentication is no longer supported. Migrate to API
         Tokens or Trusted Publishers instead. See                              
         https://pypi.org/help/#apitoken and                                    
         https://pypi.org/help/#trusted-publishers  

So I guess I need to set that up.

@fliiiix
Copy link
Contributor

fliiiix commented Jan 18, 2024

Yep thats this part

main thing that might be confusing is that the twine user is __token__ and you need to register and api token on pypi https://pypi.org/manage/account/

@fliiiix
Copy link
Contributor

fliiiix commented Jan 18, 2024

Yes setup.py is deprecated but will probably still work fine for some time. For an example migration you can take a look at something like this timofurrer/pandoc-plantuml-filter@d622397

and then you can build with something like this ->

python -m pip install --upgrade pip
python -m pip install build
python -m build

timofurrer/pandoc-plantuml-filter@5495e79#diff-28802fbf11c83a2eee09623fb192785e7ca92a3f40602a517c011b947a1822d3R23

@jgm
Copy link
Owner

jgm commented Jan 18, 2024

OK I managed to publish.

@fliiiix
Copy link
Contributor

fliiiix commented Jan 21, 2024

@jgm i just tested it and it seems like you released a version with out the merged fix 😅

pandoc tests/sample.md -o sample.tex --filter pandoc-plantuml       
Created directory plantuml-images
Created directory plantuml-images
Created directory plantuml-images

and im on 1.5.1

pip freeze | grep pandocfilter
pandocfilters==1.5.1

is it possible to release the current master as 1.5.2 or something?

@jgm
Copy link
Owner

jgm commented Jan 21, 2024

Here's what I'm seeing after installing 1.5.1 using pip3:

% diff -u pandocfilters.py /opt/homebrew/lib/python3.11/site-packages/pandocfilters.py 

returns no differences between the version in master and the installed version.

Looking at the code in /opt/homebrew/, I see this from this PR:

    try:
        os.makedirs(imagedir, exist_ok=True)
        sys.stderr.write('Created directory ' + imagedir + '\n')

So I believe the code is in the released version -- you can confirm on your own system?
Maybe the code doesn't do what you thought?

@fliiiix
Copy link
Contributor

fliiiix commented Jan 21, 2024

Oh I see 😅 the log only on errors and somehow i though i saw that the sys.stderr.write('Created directory ' + imagedir + '\n') log is gone but apparently the second part was just in my mind

would you be open to remove this log on success? (i can provide a MR if required)

@jgm
Copy link
Owner

jgm commented Jan 22, 2024

Sorry I am not sure I understand. Can you give more details?

@fliiiix
Copy link
Contributor

fliiiix commented Jan 22, 2024

No worries i will provide a MR then we can discuss it there :), btw did you had a chance to look at the MR for the pipeline? #106

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.

4 participants