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.

Sign up for GitHub

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

Jump to bottom

Project name added to user-agent #678

Merged
merged 5 commits into from
Jan 31, 2024
Merged

Project name added to user-agent #678

merged 5 commits into from
Jan 31, 2024

Conversation

joereuss12
Copy link
Contributor

For downloads, if the ProjectName is present in a job ad for the plugin, it is added to the user-agent in the http request header.

@joereuss12 joereuss12 added this to the v7.5.0 milestone Jan 19, 2024
@joereuss12 joereuss12 requested a review from bbockelm January 19, 2024 17:11
@joereuss12
Copy link
Contributor Author

@bbockelm I currently have it so that the user-agent is set to the project name if it is present for downloads. I do have two questions:

  1. Should I also add this to the header for uploads too on top of downloads?
  2. I cannot seem to find this ProjectName as a jobad attribute from when running jobs and even in the condor manual, I'm not sure how it is populated, is this a custom classad?

@joereuss12 joereuss12 linked an issue Jan 19, 2024 that may be closed by this pull request
Automatically add project name to the user agent #445
Closed
@bbockelm bbockelm requested review from jhiemstrawisc and removed request for bbockelm January 30, 2024 18:51
@joereuss12
Project name added to user-agent
e0882c7 
For downloads, if the ProjectName is present in a job ad, it is added to
the user-agent header
@joereuss12
Check nil/if ProjectName exists
48e876f 
Check these before adding to the user-agent in the header.
@joereuss12
Added user-agent header for uploads
b3a54bd
@joereuss12
Payload properly populated
431807b 
The user-agent header was not actually populated due to two things:
1. The regex within `parse_job_ad` was incorrect, and only looking at
   the very first line of the job_ad file it seems
2. `parse_job_ad` was not properly populating the payload it was passed,
   it now accepts a pointer to a payload so it is properly populated
@joereuss12 joereuss12 force-pushed the project-name-to-user-agent-branch branch from e4c4b6f to 431807b Compare January 31, 2024 16:23
jhiemstrawisc
jhiemstrawisc reviewed Jan 31, 2024
View reviewed changes
Copy link
Member

@jhiemstrawisc jhiemstrawisc left a comment

Choose a reason for hiding this comment

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

LGTM, and we tested by hand together. I still can't explain the weird issue with the RegEx, but I'm okay with saying "this seems to work and there's a comment explaining why we made the choice."

@jhiemstrawisc jhiemstrawisc merged commit 2da69dd into PelicanPlatform:main Jan 31, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhiemstrawisc jhiemstrawisc jhiemstrawisc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v7.5.0
Development

Successfully merging this pull request may close these issues.

Automatically add project name to the user agent
2 participants
@joereuss12 @jhiemstrawisc