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

Use spack in wrappers #536

Merged
merged 33 commits into from
Feb 22, 2024
Merged

Use spack in wrappers #536

merged 33 commits into from
Feb 22, 2024

Conversation

marcmengel
Copy link
Contributor

This change uses the /cvmfs/fermilab.opensciencegrid.org/packages/common Spack area to find ifdhc, via a spack environment,
rather than using the UPS pacakges in /cvmfs/fermilab.opensciencegrid.org/products/common, or looking in /grid/fermiapp
which hasn't actually worked for some time.

marcmengel and others added 30 commits February 16, 2024 14:30
* If GROUP is not set in environment when fake_ifdh.getRole is called, use getExp to guess the group.  Also use fermilab group for test

* Split out getRole to make it clearer what the logic is in discovering the role

* Replaced instances where we set the GROUP env with monkeypatch so things get set back after tests are done

* Strip off any spaces/newlines in the default getExp case

* Reorganize tests into classes, since we have so many tests for each function
* Fixed pre-commit pylint stanza so we're actually running pylint on the repo

* Added disabled checks

* Fixed bin/ files to make pylint happy or disabled checks as needed

* Fixed lib/ files to make pylint happy or disabled checks as needed - part 1

* Fixed lib/ files to make pylint happy or disabled checks as needed - part 2

* After rebasing, correct outstanding pylint errors or suppress them

* PR #508 introduced a cyclic import.  I've suppressed it here, but we need to fix this before the next release

* Moved pylint check to local install

* Skip pylint only for CI

---------

Co-authored-by: Marc Mengel <mengel@fnal.gov>
* Moved pylint check to local install

* Skip pylint only for CI

* Added pylint github action

* Testing workflow

* Move ubuntu version back to support python 3.6

* Fixed pylint calls

* Disabled OrderedDict subscriptable check.  This disabling can be taken out when we upgrade python versions

* Ignore condor_vault_storer in pylint check

* Ignore condor_vault_storer in pylint check - fixed

* Remove workflow_dispatch event
* Added capability for TarfilePublisherHandler to use a fixed RCDS server.  The default is still to switch with each operation

* Between first RCDS check for existence and publish action if necessary, use the same RCDS server

* Changes to retry logic for TarfilePublisherHandler

1. Changed TarfilePublisherHandler decorator pubapi_operation so it
   accepts a parameter, always_switch_servers.  If always_switch_servers
is set to True, then upon retries, the handler will attempt to switch
servers, even if the previous state of the handler was to keep the
server fixed.

2.  TarfilePublisherHandler.publish uses (1) so that it always attempts
    different servers upon retrying.

3.  For all operations, if the first attempt is a failure, we will
    retry immediately with the next server, according to (1).  If that
next retry fails, then we will wait for RETRY_INTERVAL_SEC before the
next and any subsequent retries.

* For the fixed_server case, make sure we don't switch to an iterator that returns the wrong server constantly

* Needed to implement the retry logic in the outer loop too

* Simplified logic for whether or not we change/restore the fixed server behavior in the pubapi_operation decorators

* Make restore_fixed_server_behavior a private function

* Added some verbose statements

* Fixed bug in __restore_fixed_server_behavior_func where we were not properly storing the old dropbox server.  Also cleaned up some comments
)

* Added github action that checks that we can run make and build RPM

* Added pull_request to events that trigger this workflow

* Added build status badge for build-al9 workflow

* Run workflow at push and merged PR

Originally the plan was to run this workflow at PR open, but that
doesn't seem possible due to:
https://github.blog/changelog/2021-02-19-github-actions-workflows-triggered-by-dependabot-prs-will-run-with-read-only-permissions/

We need a repo secret to access fermitools/jobsub_lite_config.  Since we
can't do that, for now, at least run after a PR merge or any other push
to master.

* Made build steps into custom action

* Move container prep to its own action

* Revert "Move container prep to its own action"

This reverts commit fd091ff.
* Split build workflows into PR and push workflows

* Updated status badge
@marcmengel
Copy link
Contributor Author

Note, we should definitely squash-merge this, as it has been rebased/merged and looks like a lot of commits that way.

@marcmengel marcmengel linked an issue Feb 21, 2024 that may be closed by this pull request
Copy link
Collaborator

@shreyb shreyb left a comment

Choose a reason for hiding this comment

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

I just had some questions. The change itself looks good.

templates/simple/simple.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@shreyb shreyb left a comment

Choose a reason for hiding this comment

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

Marc and I discussed changes that should happen before this PR is merged. Those are in this latest review.

templates/simple/simple.sh Outdated Show resolved Hide resolved
@shreyb
Copy link
Collaborator

shreyb commented Feb 21, 2024

I just tried running all three of those commands on dunegpvm01, and it didn't work:

-bash-4.2$ . /cvmfs/fermilab.opensciencegrid.org/packages/common/setup-env.sh
-bash-4.2$ spack env list
==> 6 environments
    ifdh_env_almalinux9_current  ifdh_env_almalinux9_v2_6_20  ifdh_env_almalinux9_v2_7  ifdh_env_scientific7_current  ifdh_env_scientific7_v2_6_20  ifdh_env_scientific7_v2_7
-bash-4.2$ spack env activate ifdh_env_scientific7_current
==> Warning: could not load runtime environment due to RuntimeError: Unable to locate python command in /cvmfs/fermilab.opensciencegrid.org/packages/common/spack/v0.21.0-fermi/NULL/var/spack/environments/ifdh_env_scientific7_current/.spack-env/view/./bin

Is python missing from those ifdh environments?

@marcmengel
Copy link
Contributor Author

marcmengel commented Feb 22, 2024 via email

@shreyb
Copy link
Collaborator

shreyb commented Feb 22, 2024

No worries - it looks good to me! I'll go ahead and squash-merge this then.

@shreyb
Copy link
Collaborator

shreyb commented Feb 22, 2024

Actually - doing two quick test submissions to make sure this works onsite and offsite:

8439421.0@jobsub04.fnal.gov (Onsite)
53143895.0@jobsub03.fnal.gov (Offsite)

@shreyb
Copy link
Collaborator

shreyb commented Feb 22, 2024

Both tests worked. This is good to go.

@shreyb shreyb merged commit fcdb272 into master Feb 22, 2024
2 checks passed
@shreyb shreyb added this to the 1.7 milestone Feb 22, 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.

Migrate away from UPS
2 participants